Improve constant propagation for Mint and Smi.

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
===================================================================
--- runtime/vm/flow_graph_optimizer.cc	(revision 22973)
+++ runtime/vm/flow_graph_optimizer.cc	(working copy)
@@ -5338,38 +5338,48 @@
 }
 
 
-void ConstantPropagator::VisitBinarySmiOp(BinarySmiOpInstr* instr) {
-  const Object& left = instr->left()->definition()->constant_value();
-  const Object& right = instr->right()->definition()->constant_value();
+void ConstantPropagator::HandleBinaryOp(Definition* instr,
+                                        Token::Kind op_kind,
+                                        const Value& left_val,
+                                        const Value& right_val) {
+  const Object& left = left_val.definition()->constant_value();
+  const Object& right = right_val.definition()->constant_value();
   if (IsNonConstant(left) || IsNonConstant(right)) {
     SetValue(instr, non_constant_);

[Kevin Millikin (Google), 2013/05/22 07:50:35]

I think this is where we would support arithmetic simplifications.  Maybe add a
TODO to consider adding and subtracting 0, multiplication or division by 1,
multiplication by 0, etc.

[srdjan, 2013/05/22 15:13:33]

Added comment:
// TODO(srdjan): Add arithemtic simplifications, e.g, add with 0.

   } else if (IsConstant(left) && IsConstant(right)) {
-    if (left.IsSmi() && right.IsSmi()) {
-      const Smi& left_smi = Smi::Cast(left);
-      const Smi& right_smi = Smi::Cast(right);
-      switch (instr->op_kind()) {
+    if (left.IsInteger() && right.IsInteger()) {
+      const Integer& left_int = Integer::Cast(left);
+      const Integer& right_int = Integer::Cast(right);
+      switch (op_kind) {
         case Token::kADD:
         case Token::kSUB:
         case Token::kMUL:
         case Token::kTRUNCDIV:
         case Token::kMOD: {
-          const Object& result = Integer::ZoneHandle(
-              left_smi.ArithmeticOp(instr->op_kind(), right_smi));
+          Instance& result = Integer::ZoneHandle(
+              left_int.ArithmeticOp(op_kind, right_int));
+          result = result.Canonicalize();
           SetValue(instr, result);
           break;
         }
         case Token::kSHL:
         case Token::kSHR: {

[Kevin Millikin (Google), 2013/05/22 07:50:35]

I'm not sure about the official VM style, but I usually leave these braces off
when there are no declarations in the scope.

[srdjan, 2013/05/22 15:13:33]

Removed braces.

-          const Object& result = Integer::ZoneHandle(
-              left_smi.ShiftOp(instr->op_kind(), right_smi));
-          SetValue(instr, result);
+          if (left.IsSmi() && right.IsSmi()) {
+            Instance& result = Integer::ZoneHandle(
+                Smi::Cast(left_int).ShiftOp(op_kind, Smi::Cast(right_int)));
+            result = result.Canonicalize();
+            SetValue(instr, result);
+          } else {
+            SetValue(instr, non_constant_);
+          }
           break;
         }
         case Token::kBIT_AND:
         case Token::kBIT_OR:
         case Token::kBIT_XOR: {
-          const Object& result = Integer::ZoneHandle(
-              left_smi.BitOp(instr->op_kind(), right_smi));
+          Instance& result = Integer::ZoneHandle(
+              left_int.BitOp(op_kind, right_int));
+          result = result.Canonicalize();
           SetValue(instr, result);
           break;
         }
@@ -5385,6 +5395,11 @@
 }
 
 
+void ConstantPropagator::VisitBinarySmiOp(BinarySmiOpInstr* instr) {
+  HandleBinaryOp(instr, instr->op_kind(), *instr->left(), *instr->right());
+}
+
+
 void ConstantPropagator::VisitBoxInteger(BoxIntegerInstr* instr) {
   // TODO(kmillikin): Handle box operation.
   SetValue(instr, non_constant_);
@@ -5399,15 +5414,13 @@
 
 void ConstantPropagator::VisitBinaryMintOp(
     BinaryMintOpInstr* instr) {
-  // TODO(kmillikin): Handle binary operations.
-  SetValue(instr, non_constant_);
+  HandleBinaryOp(instr, instr->op_kind(), *instr->left(), *instr->right());
 }
 
 
 void ConstantPropagator::VisitShiftMintOp(
     ShiftMintOpInstr* instr) {
-  // TODO(kmillikin): Handle shift operations.
-  SetValue(instr, non_constant_);
+  HandleBinaryOp(instr, instr->op_kind(), *instr->left(), *instr->right());
 }