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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

This feels very fragile... both in that it's easy to get wrong and only handles a very specific pattern.



================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:1195
+        StepInst->getOperand(1) ==
+            ConstantInt::get(StepInst->getOperand(1)->getType(), 1)) {
+      StartValue = PN->getIncomingValue(1 - I);
----------------
Does this check for step 1 still make sense, given the more generic checks below?

Also, aren't we missing a check here that the GEP pointer operand is the PN? I think right now it can be unrelated...


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:1213
+                               StepConstantOffset) ||
+      !StepVariableOffsets.empty())
+    return;
----------------
This looks like a complicated way to write accumulateConstantOffset().


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:1238
+    if (!UpperScale.urem(StepConstantOffset).isZero())
+      return;
+  }
----------------
A cleaner (and more general) way to write this would be that all the scales/offset must be divisible by StepConstantOffset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152730