[PATCH] D63680: [LoopRotate + MemorySSA] Keep an <instruction-cloned instruction> map.

Alina Sbirlea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 9 16:02:21 PDT 2019


asbirlea marked an inline comment as done.
asbirlea added inline comments.


================
Comment at: lib/Transforms/Utils/LoopRotationUtils.cpp:379
+      // not whether it can be remapped to a simplified value.
+      ValueMapMSSA[Inst] = C;
     }
----------------
george.burgess.iv wrote:
> 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
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.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63680/new/

https://reviews.llvm.org/D63680





More information about the llvm-commits mailing list