[PATCH] D35010: [IRCE] Recognize loops with ne/eq latch conditions

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:853
+      // it virtually when we replaced EQ with SGT.
+      if (!DecreasedRightValueByOne) {
+        IRBuilder<> B(Preheader->getTerminator());
----------------
mkazantsev wrote:
> sanjoy wrote:
> > I'm missing a link here.  IIUC:
> > 
> >  - You started with the backedge taken condition as `!(++i == len)`
> >  - `!(++i == len)` is equivalent to `!(++i >s (len - 1))`, after checking `len - 1` does not overflow (this is the new step you added)
> >  - `!(++i >s (len - 1))` is equivalent to `(++i s<= (len - 1))`
> >  - `(++i s<= (len - 1))` is equivalent to `(++i s< len)` after checking that `(len - 1) + 1` does not overflow
> > 
> > Where does `DecreasedRightValueByOne` come in?
> In case 2. After we go from `(++i == length)` to `(++i >s (len - 1))`, the new `RighValue` we are working with is actually `len - 1`. We don't need to create such an instruction, though. `RightValue` is only used to calculate `RightValue + 1`. Knowing that we have decreased the old `RightValue` by 1, we could just not add this 1 and use the old value of `len` instead.
I see -- do you mind (when you have time) refactoring this code a bit to make things like this more clear?  For instance, it is confusing that we have `RightValue` and `RightSCEV` that are not in sync at certain points in the control flow.

One way to do this would be to have `LoopStructure` contain a "virtual" version of the latch branch (i.e. `ICmpInst::Predicate Pred`, `const SCEV *LHS`, `const SCEV *RHS`, `BasicBlock *TrueBranch`, `BasicBlock *FalseBranch`) and always rewrite the latch branch to be this "canonical" version during IRCE (after the gating checks have passed).



https://reviews.llvm.org/D35010