[PATCH] [LSR] canonicalize Prod*(1<<C) to Prod<<C

[David Majnemer]

================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:31
@@ -29,3 +30,3 @@
 
 using namespace llvm;
 
----------------
I'd add `using namespace PatternMatch;` here.

================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:771
@@ +770,3 @@
+      const APInt *RHS;
+      if (PatternMatch::match(W, PatternMatch::m_Power2(RHS))) {
+        // Canonicalize Prod*(1<<C) to Prod<<C.
----------------
majnemer wrote:
> Seeing the continue here is a little confusing, can we just make it an if/else?
With the `using` declaration, we can remove the scope qualifiers here.

================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:771-777
@@ -771,3 +770,9 @@
+      const APInt *RHS;
+      if (PatternMatch::match(W, PatternMatch::m_Power2(RHS))) {
+        // Canonicalize Prod*(1<<C) to Prod<<C.
+        Prod = InsertBinop(Instruction::Shl, Prod,
+                           ConstantInt::get(Ty, RHS->logBase2()));
+        continue;
+      }
       Prod = InsertBinop(Instruction::Mul, Prod, W);
     }
----------------
Seeing the continue here is a little confusing, can we just make it an if/else?

================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:773-774
@@ +772,4 @@
+        // Canonicalize Prod*(1<<C) to Prod<<C.
+        Prod = InsertBinop(Instruction::Shl, Prod,
+                           ConstantInt::get(Ty, RHS->logBase2()));
+        continue;
----------------
I'd use `Constant::getIntegerValue` instead as it handles the case where `Ty` is a vector type.

http://reviews.llvm.org/D10448

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/