[PATCH] D51718: Update MemorySSA in LoopRotate.

David Greene via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 20 18:22:18 PDT 2018


greened added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopRotation.cpp:49
+  bool Changed = LoopRotation(&L, &AR.LI, &AR.TTI, &AR.AC, &AR.DT, &AR.SE,
+                              MSSAU.hasValue() ? MSSAU.getPointer() : nullptr,
+                              SQ, false, Threshold, false);
----------------
asbirlea wrote:
> greened wrote:
> > Why not just pass the Optional?
> I'm inclined to prefer using the pointer just due to consistency with the other passes + the hope that this won't be Optional forever :).
> 
> If you or other reviewers strongly prefer passing the Optional, please let me know and I'll update here and in all other passes.
I don't have a strong position on this, just throwing it out as an idea.  Using an optional does eliminate the ternary operations and would make the code a bit cleaner.

How expensive is MemorySSA?  I'm not sure we can guarantee it will be available so it might always have to be optional.


Repository:
  rL LLVM

https://reviews.llvm.org/D51718





More information about the llvm-commits mailing list