[PATCH] D42835: [SCEV] Make isLoopEntryGuardedByCond a bit smarter

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:9109
+  if (Pred != NonStrictPredicate &&
+      isLoopEntryGuardedByCond(L, ICmpInst::ICMP_NE, LHS, RHS) &&
+      isLoopEntryGuardedByCond(L, NonStrictPredicate, LHS, RHS))
----------------
mkazantsev wrote:
> sanjoy wrote:
> > I'm sorry I didn't notice this before, but I think this is better to "fuse" this into the iteration we're already doing instead of recursing (which will walk the dominating checks again).  Perhaps extract out a lambda that you call instead of `isImpliedCond` in this function that has this bit of logic too?
> `isLoopEntryGuardedByCond` does traverse over dominating conditions. If we do this, we will have `O(N^2)` with `N` being number of dominating blocks.
> 
> I guess that your point here is that we can be able to prove one fact from assumption and another one from dominating guard. I'm totally fine with that, but I'd rather not fuse it into this loop. I'll give it some thought.
> 
> How about merging it as is an then making this improvement in separate patch?
> I guess that your point here is that we can be able to prove one fact from assumption and another one from dominating guard. I'm totally fine with that, but I'd rather not fuse it into this loop.

Yes -- you could do something like: if you can't prove A < B but can prove A != B, simplify the predicate you're trying to prove to A <= B and continue.

> How about merging it as is an then making this improvement in separate patch?

Unless you're blocked on this patch, I'd rather do the right thing now than check in something with the intent to remove it later.


https://reviews.llvm.org/D42835