[PATCH] D98222: [SCEV] Use trip count information to improve shift recurrence ranges

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:5655
+  if (!L || L->getHeader() != P->getParent())
+    return CR;
+
----------------
`LI.isLoopHeader(P->getParent())`?
I also wonder why is that needed. The implementation of `matchSimpleRecurrence` cannot recognize something which is not a header phi, because there is no other way to have BO as operand of Phi and vice versa at the same time. I guess this may be turned into assert after `matchSimpleRecurrence`.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:5663
+  // TODO: Handle ashr and lshr cases to increase minimum value reported
+  if (BO->getOpcode() != Instruction::Shl || BO->getOperand(0) != P)
+    return CR;
----------------
Looks like it's better split it into two checks, as `ashr` and `lshr` will still need `BO->getOperand(0) != P`.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:5665
+    return CR;
+
+  unsigned TC = getSmallConstantMaxTripCount(L);
----------------
Are we missing a check that another operand is loop-invariant, or it is implied by something?


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

https://reviews.llvm.org/D98222