Merge ConstantInstr -> UnboxDouble to UnboxedConstant. Reduces register usage and allows for variou…

runtime/vm/intermediate_language.cc

Index: runtime/vm/intermediate_language.cc
===================================================================
--- runtime/vm/intermediate_language.cc	(revision 35632)
+++ runtime/vm/intermediate_language.cc	(working copy)
@@ -325,6 +325,14 @@
 }
 
 
+UnboxedConstantInstr::UnboxedConstantInstr(const Object& value)
+    : ConstantInstr(value), constant_address_(NULL) {
+  // Only doubles supported for now.
+  ASSERT(value.IsDouble());
+  constant_address_ =
+      FlowGraphBuilder::FindDoubleConstant(Double::Cast(value).value());
+}
+
 // Returns true if the value represents a constant.
 bool Value::BindsToConstant() const {
   return definition()->IsConstant();
@@ -1224,7 +1232,6 @@
       return ToIntegerConstant(value->definition()->AsUnboxDouble()->value(),
                                result);
     }
-
     return false;
   }
 
@@ -1602,8 +1609,19 @@
 
 Definition* UnboxDoubleInstr::Canonicalize(FlowGraph* flow_graph) {

[Florian Schneider, 2014/05/01 18:34:16]

Maybe add

if (!HasUses()) return NULL;

so that Unbox operations that are not used anymore after canonicalization get
removed in a following pass.

[srdjan, 2014/05/01 20:33:04]

Done.

   // Fold away UnboxDouble(BoxDouble(v)).
-  BoxDoubleInstr* defn = value()->definition()->AsBoxDouble();
-  return (defn != NULL) ? defn->value()->definition() : this;
+  BoxDoubleInstr* box_defn = value()->definition()->AsBoxDouble();
+  if (box_defn != NULL) {
+    return box_defn->value()->definition();
+  }
+
+  ConstantInstr* c = value()->definition()->AsConstant();
+  if ((c != NULL) && c->value().IsDouble()) {
+    UnboxedConstantInstr* uc = new UnboxedConstantInstr(c->value());
+    flow_graph->InsertBefore(this, uc, this->env(), Definition::kValue);

[Florian Schneider, 2014/05/01 18:34:16]

UnboxedConstant should not need an environment, since it can't deoptimize and
can't become a deoptimization target. I'd pass NULL instead of this->env().

[srdjan, 2014/05/01 20:33:04]

Done.

+    return uc;

[Florian Schneider, 2014/05/01 18:34:16]

Make sure that the UnboxDoubleInstr get removed by the optimizer after the new
UnboxedConstant is inserted. It should not have any uses after this Canonicalize
step, so it can be just removed from the graph.

[srdjan, 2014/05/01 20:33:04]

I checked it visually in disassembled output/flow graph. Any other way?

+  }
+
+  return this;
 }