[PATCH] D42953: [SCEV] Make getPostIncExpr guaranteed to return AddRec

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:10221
+  assert(getNumOperands() > 1 && "AddRec with zero step?");
+  const SCEV *Step = getStepRecurrence(SE);
+  // There is a temptation to just call SE.getAddExpr(this, Step), but in this
----------------
This is unnecessary -- the step for `{A,+,B,+,C}` will always be `{B,+,C}` so we should be able to use `Ops.push_back(SE.getAddExpr(getOperand(i), getOperand(i+1)));` in the loop below.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:10223
+  // There is a temptation to just call SE.getAddExpr(this, Step), but in this
+  // case we cannot guarantee that the value returned will be an AddRec. So we
+  // make it explicitly. Given that the last op of this is non zero, it will
----------------
Also state why we can't guarantee it (SCEV does not simplify to fixed point etc.).


================
Comment at: lib/Analysis/ScalarEvolution.cpp:10224
+  // case we cannot guarantee that the value returned will be an AddRec. So we
+  // make it explicitly. Given that the last op of this is non zero, it will
+  // also be non-zero if we add this and Step one by one operand. This does give
----------------
I didn't quite understand what you meant by "last op of this is non zero".


https://reviews.llvm.org/D42953