[PATCH] D34025: [SCEV] Teach SCEVExpander to expand BinPow

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm with nits



================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:767
+    while (E != OpsAndLoops.end() && *I == *E && Exponent != MaxExponent) {
+      ++Exponent; ++E;
+    }
----------------
Avoid braces here.

You could also write this loop using `find_if`.

Finally I'd perhaps suggest using a `uint64_t` for `Exponent` so that an overflow is practically impossible.


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:775
+    Value *Result = nullptr;
+    if (Exponent & 1u)
+      Result = P;
----------------
Can you get rid of this conditional and instead start `BinExp` from `1`?


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:792
       // This is the first operand. Just expand it.
-      Prod = expand(Op);
+      Prod = ExpandOpBinPowN();
     } else if (Op->isAllOnesValue()) {
----------------
It is a bit hard to read `ExpandOpBinPowN()` and tell what all local state the call touches.  Do you mind extracting out a static helper function so that the data flow is more obvious?


https://reviews.llvm.org/D34025