[PATCH] D27939: [LICM] Report failing to hoist a load with an invariant address

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:477
+        pointerInvalidatedByLoop(LI->getOperand(0), Size, AAInfo, CurAST);
+    if (ORE && Invalidated && CurLoop->isLoopInvariant(LI->getPointerOperand()))
+      ORE->emit(OptimizationRemarkMissed(
----------------
anemet wrote:
> anemet wrote:
> > hfinkel wrote:
> > > Can we hoist a load without a loop-invariant address? It seems like we shouldn't be making aliasing queries for loads we can't possibly hoist, and so I don't understand why we specifically need the `CurLoop->isLoopInvariant` check here.
> > No, you're right.  The call to this is guarded with hasLoopInvariantOperands.
> Actually, I take this back.  We need the check.  See the new comment in the new version of the patch and also the new testcase for an example.
Okay, but I wonder if this points to a larger problem. Maybe we just need a FIXME on this comment for now, but I'm thinking that this invariance check is cheap and the other checks (i.e. aliasing queries, dereferenceablility checks, etc.) are, at least in some cases, expensive. Thus, this guard really belongs elsewhere. We can sink something with a non-loop-invariant store either, can we?


https://reviews.llvm.org/D27939