[PATCH] D28308: [SCEV] Model ashr(shl(x, n), m) as mul(x, 2^(n-m)) when n > m

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:5266
+          uint64_t Amt = BitWidth - CI->getZExtValue();
+          Value *LOp1 = L->getOperand(1);
+          if (Amt == BitWidth) {
----------------
Maybe rename Amt and LOp1 to something which actually describes the values?


================
Comment at: lib/Analysis/ScalarEvolution.cpp:5267
+          Value *LOp1 = L->getOperand(1);
+          if (Amt == BitWidth) {
+            if (LOp1 == BO->RHS)
----------------
Maybe move this check earlier?  You could do it before you even check that the LHS of the ashr is another shift: just "if (CI->isNullValue()) return getSCEV(BO->LHS)".


================
Comment at: lib/Analysis/ScalarEvolution.cpp:5290
+
+          uint64_t AShrAmt = CI->getZExtValue();
+          uint64_t LShAmt = LCI->getZExtValue();
----------------
Hoist this variable definition, so you don't call CI->getZExtValue() twice.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:5298
+              LShAmt - AShrAmt > Amt)
+            break;
+
----------------
"LShAmt - AShrAmt >= Amt" is equivalent to "LShAmt - AShrAmt >= BitWidth - AshrAmt", which is equivalent to "LShAmt >= BitWidth", which you already check earlier.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:5300
+
+          uint64_t Mul = 1 << (LShAmt - AShrAmt);
+          const SCEV *MulSCEV = getSCEV(ConstantInt::get(TruncTy, Mul));
----------------
APInt::getOneBitSet?  (The amount of the multiply could overflow "int".)


Repository:
  rL LLVM

https://reviews.llvm.org/D28308