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

[Jingyue Wu]

Thanks for the review, David.


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

================
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:
> 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.
done

================
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);
     }
----------------
jingyue wrote:
> majnemer wrote:
> > 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.
> done
done

================
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;
----------------
majnemer wrote:
> I'd use `Constant::getIntegerValue` instead as it handles the case where `Ty` is a vector type.
Vector types are not SCEVable, so SCEV::getType() never returns a vector type. But it doesn't harm to add an assert.

http://reviews.llvm.org/D10448

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