[PATCH] D36215: [IRCE] Identify loops with latch comparison against current IV value

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev marked 3 inline comments as done.
mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:848
+  const SCEV *IndVarStart = nullptr;
+  if (isa<PHINode>(LeftValue) &&
+      cast<PHINode>(LeftValue)->getParent() == Header)
----------------
mkazantsev wrote:
> anna wrote:
> > I'm not sure if this is enough to identify that it's from the current IV. 
> > 
> > What if we had some "noop" operations on that phi node, and the result of that operation was used in the comparison? Wouldn't we incorrectly mark as this is from iv.next, whereas it's infact from the current IV.
> > ```
> > header:
> > iv = phi [0, entry] [iv.next, latch]
> > ...
> > 
> > latch:
> > iv.trunc =  trunc iv to 16
> > iv.next = iv + 1
> > %cmp = slt iv.trunc, 100
> > br %cmp, header, exit
> > ```
> > It could be bitcast or trunc or any other such operations. I think we need to "look through these operations" rather than directly comparing against a phi node.
> > 
> > Please add test cases as well.
> > 
> In current implementation, such case will be still recognized as comparison against `iv.next`. That's an interesting point, I need to think on it. Thanks for noticing that!
I checked that they follow by `iv.next` branch and get rejected on iteration range safety stage. Given that this behavior also exists without this patch, this doesn't seem to produce bugs. Added a TODO to handle these cases by the first branch as a scope improvement.


https://reviews.llvm.org/D36215