[PATCH] D13259: LLE 6/6: Add LoopLoadElimination pass

[Renato Golin via llvm-commits]

rengolin added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopLoadElimination.cpp:319
@@ +318,3 @@
+    SmallSet<Value *, 4> PtrsWrittenOnFwdingPath =
+        computePointersWrittenOnForwardingPath(Candidates);
+
----------------
anemet wrote:
> rengolin wrote:
> > Silly nitpick: I originally thought this function would actually find the forward dependencies, not just find the candidates. Maybe changing the name to findPointers... would be more clear.
> This comment may have shifted or I am just confused.  Can you please elaborate what you mean?
Sorry. It's about:

    computePointersWrittenOnForwardingPath(Candidates);

You don't use it elsewhere, and it doesn't seem important on its own. I'm guessing you pulled it out because of collectMemchecks' size. This nitpick was just a personal preference to call it "find" or "collect" instead of "compute", since that's all it's doing. But it's not important.

================
Comment at: lib/Transforms/Scalar/LoopLoadElimination.cpp:368
@@ +367,3 @@
+    Value *Ptr = Cand.Load->getPointerOperand();
+    auto *PtrSCEV = cast<SCEVAddRecExpr>(SE->getSCEV(Ptr));
+    auto *PH = L->getLoopPreheader();
----------------
anemet wrote:
> rengolin wrote:
> > would this be meaningful?
> > 
> >     assert(PtrSCEV)
> cast has an implicit assert, no?
ah, yes, of course!


http://reviews.llvm.org/D13259