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

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 18 20:26:43 PDT 2017


mkazantsev added inline comments.


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:792
       // This is the first operand. Just expand it.
-      Prod = expand(Op);
+      Prod = ExpandOpBinPowN();
     } else if (Op->isAllOnesValue()) {
----------------
sanjoy wrote:
> 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?
We call internal SCEVExpander method (like InsertBinop) inside. Do we really want a static helper that takes "this" as argument?

I think the better way to make the data flow obvious here is to capture the required objects explicitly.


https://reviews.llvm.org/D34025





More information about the llvm-commits mailing list