[PATCH] D152730: [ConstraintElim] Add A < B if A is an increasing phi for A != B.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a subscriber: ldionne.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:1149
+    auto *N = DT.getNode(Succ);
+    if (NumIn == N->getDFSNumIn() && NumOut == N->getDFSNumOut()) {
+      InLoopSucc = Succ;
----------------
antoniofrighetto wrote:
> Very minor, but one may find this slightly more readable?
> ```
>   if (NumIn == N->getDFSNumIn() && NumOut == N->getDFSNumOut())
>     InLoopSucc = Succ;
>   else
>     LoopExitSucc = Succ;
> ```
Simplified, thanks!


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:1171
+        StepInst->getOperand(1) ==
+            ConstantInt::get(StepInst->getOperand(1)->getType(), 1)) {
+      StartValue = PN->getIncomingValue(1 - I);
----------------
nikic wrote:
> This does not guarantee a step of 1, because you're missing the multiplication with the type alloc size here. And if you add that, it likely won't support the cases you're interested in any more.
> 
> It would probably be easier to handle integer inductions, pointers are more complicated.
Yep! I *think*  GEPs with element type of `i8` should be fine unconditionally, for others, it should be fine if the GEP we compare to has the same element type as the step GEP.

The main motivation  is hardening libc++ where this allows us to remove checks for loops using iterators which should be quite high impact.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152730/new/

https://reviews.llvm.org/D152730