Use UnboxedInt32 and UnboxedUint32 representation for LoadIndexedInstr

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
diff --git a/runtime/vm/flow_graph_optimizer.cc b/runtime/vm/flow_graph_optimizer.cc
index f5333546f2ed98d51cc45fded39181aab427b875..f809304766805af8f885eba21705d3a02770da39 100644
--- a/runtime/vm/flow_graph_optimizer.cc
+++ b/runtime/vm/flow_graph_optimizer.cc
@@ -75,6 +75,20 @@ static bool CanConvertUnboxedMintToDouble() {
 #endif
 }
 
+
+static bool CanConvertUnboxedInt32ToDouble() {
+#if defined(TARGET_ARCH_IA32)
+  return true;
+#elif defined(TARGET_ARCH_ARM)
+  return true;
+#elif defined(TARGET_ARCH_X64)
+  return true;
+#else
+  return false;

[Vyacheslav Egorov (Google), 2014/10/02 17:14:04]

How hard it is to support Int32ToDouble() on MIPS and ARM64? I suspect it's a
very simple instruction sequence (already available in both back ends as part of
UnboxDoubleInstr).

Please consider supporting it instead of including this check.

[Cutch, 2014/10/02 22:41:12]

Done.

+#endif
+}
+
+
 // Optimize instance calls using ICData.
 void FlowGraphOptimizer::ApplyICData() {
   VisitBlocks();
@@ -628,6 +642,8 @@ void FlowGraphOptimizer::InsertConversion(Representation from,
     converted = new(I) UnboxIntegerInstr(use->CopyWithType(), deopt_id);
   } else if ((from == kUnboxedMint) && (to == kTagged)) {
     converted = new(I) BoxIntegerInstr(use->CopyWithType());
+  } else if ((from == kUnboxedUint32) && (to == kTagged)) {
+    converted = new(I) BoxUint32Instr(use->CopyWithType());
   } else if (IsUnboxedInteger(from) && IsUnboxedInteger(to)) {
     const intptr_t deopt_id = (to == kUnboxedInt32) && (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
@@ -636,7 +652,16 @@ void FlowGraphOptimizer::InsertConversion(Representation from,
                                                 use->CopyWithType(),
                                                 deopt_id);
   } else if ((from == kUnboxedInt32) && (to == kUnboxedDouble)) {
-    converted = new Int32ToDoubleInstr(use->CopyWithType());
+    const intptr_t deopt_id = (deopt_target != NULL) ?
+        deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
+    if (CanConvertUnboxedInt32ToDouble()) {
+      converted = new Int32ToDoubleInstr(use->CopyWithType());
+    } else {
+      BoxInt32Instr* boxed = new(I) BoxInt32Instr(use->CopyWithType());
+      use->BindTo(boxed);
+      InsertBefore(insert_before, boxed, NULL, FlowGraph::kValue);
+      converted = new(I) UnboxDoubleInstr(new(I) Value(boxed), deopt_id);
+    }
   } else if ((from == kTagged) && (to == kUnboxedInt32)) {
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
@@ -1355,7 +1380,16 @@ bool FlowGraphOptimizer::InlineSetIndexed(
        RawObject::IsTypedDataViewClassId(array_cid) ||
        RawObject::IsExternalTypedDataClassId(array_cid)) ? kNoStoreBarrier
                                                          : kEmitStoreBarrier;
-  if (!value_check.IsNull()) {
+
+  // If the stored value is an integer that is already unboxed as
+  // the StoreIndexedInstr expects, skip the class check.
+  bool is_already_unboxed =
+      ((array_cid == kTypedDataInt32ArrayCid) &&
+       (stored_value->representation() == kUnboxedInt32)) ||
+      ((array_cid == kTypedDataUint32ArrayCid) &&
+       (stored_value->representation() == kUnboxedUint32));
+
+  if (!is_already_unboxed && !value_check.IsNull()) {

[Vyacheslav Egorov (Google), 2014/10/02 17:14:04]

I am somewhat concerned about fragility of this.

Have you tried just making it: 

if ((array_cid != kTypedDataInt32ArrayCid) && (array_cid !=
kTypedDataUint32ArrayCid) ... ) here

1. UnboxInt32Instr should have class checks built in

2. There is a canonicalization rule doing Unbox(Box(v)) -> v 

[Cutch, 2014/10/02 22:41:12]

Done.

     // No store barrier needed because checked value is a smi, an unboxed mint,
     // an unboxed double, an unboxed Float32x4, or unboxed Int32x4.
     needs_store_barrier = kNoStoreBarrier;
@@ -1375,7 +1409,7 @@ bool FlowGraphOptimizer::InlineSetIndexed(
                                     stored_value,
                                     NULL,
                                     FlowGraph::kValue);
-  } else if (array_cid == kTypedDataInt32ArrayCid) {
+  } else if (!is_already_unboxed && (array_cid == kTypedDataInt32ArrayCid)) {
     stored_value = new(I) UnboxInt32Instr(
         new(I) Value(stored_value),
         call->deopt_id());
@@ -1384,7 +1418,7 @@ bool FlowGraphOptimizer::InlineSetIndexed(
                                     stored_value,
                                     call->env(),
                                     FlowGraph::kValue);
-  } else if (array_cid == kTypedDataUint32ArrayCid) {
+  } else if (!is_already_unboxed && (array_cid == kTypedDataUint32ArrayCid)) {
     stored_value = new(I) UnboxUint32Instr(
         new(I) Value(stored_value),
         call->deopt_id());