This is an archive of the discontinued LLVM Phabricator instance.

[LoopRotate + MemorySSA] Keep an <instruction-cloned instruction> map.
ClosedPublic

Authored by asbirlea on Jun 21 2019, 4:17 PM.

Details

Summary

The map kept in loop rotate is used for instruction remapping, in order
to simplify the clones of instructions. Thus, if an instruction can be
simplified, its simplified value is placed in the map, even when the
clone is added to the IR. MemorySSA in contrast needs to know about that
clone, so it can add an access for it.
To resolve this: keep a different map for MemorySSA.

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.Jun 21 2019, 4:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2019, 4:17 PM
Herald added subscribers: Prazek, jlebar. · View Herald Transcript

thanks for this!

just a single optional style nit.

lib/Transforms/Utils/LoopRotationUtils.cpp
379 ↗(On Diff #206097)

are these all intended to turn into inserts, or is the intent to overwrite old values, too?

in the former case, please do the (admittedly obnoxious to type)

bool Inserted = ValueMapMSSA.insert({Inst, C}).second;
(void)Inserted;
assert(Inserted);

unless Foo[X] = Bar; is by far the dominant style in this file

This revision is now accepted and ready to land.Jun 24 2019, 2:13 PM
asbirlea marked an inline comment as done.Jul 9 2019, 4:02 PM
asbirlea added inline comments.
lib/Transforms/Utils/LoopRotationUtils.cpp
379 ↗(On Diff #206097)

I believe all are inserts, but I'm inclined to keep as is, because this holds true for the other ValueMap as well, and the assignment style is used there.

I'd do a refactor NFC for both in a separate patch instead.

This revision was automatically updated to reflect the committed changes.