Use UnboxedInt32 and UnboxedUint32 representation for LoadIndexedInstr

runtime/vm/flow_graph_optimizer.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/flow_graph_optimizer.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
 #include "vm/cpu.h"
 #include "vm/dart_entry.h"
@@ skipping 57 matching lines @@
   return true;
 #else
   // ARM does not have a short instruction sequence for converting int64 to
   // double.
   // TODO(johnmccutchan): Investigate possibility on MIPS once
   // mints are implemented there.
   return false;
 #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();
 }
 
 
 // Optimize instance calls using cid.  This is called after optimizer
 // converted instance calls to instructions. Any remaining
 // instance calls are either megamorphic calls, cannot be optimized or
 // have no runtime type feedback collected.
@@ skipping 533 matching lines @@
 
   Definition* converted = NULL;
   if ((from == kTagged) && (to == kUnboxedMint)) {
     ASSERT((deopt_target != NULL) ||
            (use->Type()->ToCid() == kUnboxedMint));
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
     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;
     converted = new(I) UnboxedIntConverterInstr(from,
                                                 to,
                                                 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;
     converted = new UnboxInt32Instr(use->CopyWithType(), deopt_id);
   } else if ((from == kUnboxedInt32) && (to == kTagged)) {
     converted = new BoxInt32Instr(use->CopyWithType());
   } else if ((from == kTagged) && (to == kUnboxedUint32)) {
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
     converted = new UnboxUint32Instr(use->CopyWithType(), deopt_id);
@@ skipping 698 matching lines @@
                                      &array,
                                      index,
                                      &cursor);
 
   // Check if store barrier is needed. Byte arrays don't need a store barrier.
   StoreBarrierType needs_store_barrier =
       (RawObject::IsTypedDataClassId(array_cid) ||
        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;
     Instruction* check = GetCheckClass(
         stored_value, value_check, call->deopt_id(), call->token_pos());
     cursor = flow_graph()->AppendTo(cursor,
                                     check,
                                     call->env(),
                                     FlowGraph::kEffect);
   }
 
   if (array_cid == kTypedDataFloat32ArrayCid) {
     stored_value =
         new(I) DoubleToFloatInstr(
             new(I) Value(stored_value), call->deopt_id());
     cursor = flow_graph()->AppendTo(cursor,
                                     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());
     stored_value->AsUnboxIntN()->mark_truncating();
     cursor = flow_graph()->AppendTo(cursor,
                                     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());
     ASSERT(stored_value->AsUnboxIntN()->is_truncating());
     cursor = flow_graph()->AppendTo(cursor,
                                     stored_value,
                                     call->env(),
                                     FlowGraph::kValue);
   }
 
@@ skipping 8722 matching lines @@
 
   // Insert materializations at environment uses.
   for (intptr_t i = 0; i < exits_collector_.exits().length(); i++) {
     CreateMaterializationAt(
         exits_collector_.exits()[i], alloc, alloc->cls(), *slots);
   }
 }
 
 
 }  // namespace dart