Box values of LoadCodeUnits that fall outside of smi range on 32bit platforms.

runtime/vm/intermediate_language_arm.cc

Index: runtime/vm/intermediate_language_arm.cc
diff --git a/runtime/vm/intermediate_language_arm.cc b/runtime/vm/intermediate_language_arm.cc
index 929be42276b7eb63cef0675500ae185dc554435b..d93d8d7d9b8c32e7622b5a047fcfd4d890c563e3 100644
--- a/runtime/vm/intermediate_language_arm.cc
+++ b/runtime/vm/intermediate_language_arm.cc
@@ -1328,98 +1328,6 @@ void LoadIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
 }
 
 
-Representation LoadCodeUnitsInstr::representation() const {
-  switch (class_id()) {
-    case kOneByteStringCid:
-    case kExternalOneByteStringCid:
-    case kTwoByteStringCid:
-    case kExternalTwoByteStringCid:
-      // TODO(zerny): kUnboxedUint32 could be a better choice.
-      return can_pack_into_smi() ? kTagged : kUnboxedMint;
-    default:
-      UNIMPLEMENTED();
-      return kTagged;
-  }
-}
-
-
-LocationSummary* LoadCodeUnitsInstr::MakeLocationSummary(Isolate* isolate,
-                                                         bool opt) const {
-  const intptr_t kNumInputs = 2;
-  const intptr_t kNumTemps = 0;
-  LocationSummary* summary = new(isolate) LocationSummary(
-      isolate, kNumInputs, kNumTemps, LocationSummary::kNoCall);
-  summary->set_in(0, Location::RequiresRegister());
-  summary->set_in(1, Location::RequiresRegister());
-
-  if (representation() == kUnboxedMint) {
-    summary->set_out(0, Location::Pair(Location::RequiresRegister(),
-                                    Location::RequiresRegister()));
-  } else {
-    ASSERT(representation() == kTagged);
-    summary->set_out(0, Location::RequiresRegister());
-  }
-
-  return summary;
-}
-
-
-void LoadCodeUnitsInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
-  const Register array = locs()->in(0).reg();
-  const Location index = locs()->in(1);
-
-  Address element_address = __ ElementAddressForRegIndex(
-        true,  IsExternal(), class_id(), index_scale(), array, index.reg());
-  // Warning: element_address may use register IP as base.
-
-  if (representation() == kUnboxedMint) {
-    ASSERT(locs()->out(0).IsPairLocation());
-    PairLocation* result_pair = locs()->out(0).AsPairLocation();
-    Register result1 = result_pair->At(0).reg();
-    Register result2 = result_pair->At(1).reg();
-    switch (class_id()) {
-      case kOneByteStringCid:
-      case kExternalOneByteStringCid:
-        ASSERT(element_count() == 4);
-        __ ldr(result1, element_address);
-        __ eor(result2, result2, Operand(result2));
-        break;
-      case kTwoByteStringCid:
-      case kExternalTwoByteStringCid:
-        ASSERT(element_count() == 2);
-        __ ldr(result1, element_address);
-        __ eor(result2, result2, Operand(result2));
-        break;
-      default:
-        UNREACHABLE();
-    }
-  } else {
-    ASSERT(representation() == kTagged);
-    Register result = locs()->out(0).reg();
-    switch (class_id()) {
-      case kOneByteStringCid:
-      case kExternalOneByteStringCid:
-        switch (element_count()) {
-          case 1: __ ldrb(result, element_address); break;
-          case 2: __ ldrh(result, element_address); break;
-          default: UNREACHABLE();
-        }
-        __ SmiTag(result);
-        break;
-      case kTwoByteStringCid:
-      case kExternalTwoByteStringCid:
-        ASSERT(element_count() == 1);
-        __ ldrh(result, element_address);
-        __ SmiTag(result);
-        break;
-      default:
-        UNREACHABLE();
-        break;
-    }
-  }
-}
-
-
 Representation StoreIndexedInstr::RequiredInputRepresentation(
     intptr_t idx) const {
   // Array can be a Dart object or a pointer to external data.
@@ -1955,6 +1863,116 @@ class BoxAllocationSlowPath : public SlowPathCode {
 };
 
 
+
+
+LocationSummary* LoadCodeUnitsInstr::MakeLocationSummary(Isolate* isolate,
+                                                         bool opt) const {
+  const bool kMightBox = (representation() == kTagged) && !can_pack_into_smi();

[Florian Schneider, 2014/11/18 12:20:24]

Since this is not a compile-time constant, I'd rename it:

s/kMightBox/might_box/

[zerny-google, 2014/11/18 12:34:04]

Yes. Done here and in _ia32.cc

+  const intptr_t kNumInputs = 2;
+  const intptr_t kNumTemps = kMightBox ? 1 : 0;
+  LocationSummary* summary = new(isolate) LocationSummary(
+      isolate, kNumInputs, kNumTemps,
+      kMightBox ? LocationSummary::kCallOnSlowPath : LocationSummary::kNoCall);
+  summary->set_in(0, Location::RequiresRegister());
+  summary->set_in(1, Location::RequiresRegister());
+
+  if (kMightBox) {
+    summary->set_temp(0, Location::RequiresRegister());
+  }
+
+  if (representation() == kUnboxedMint) {
+    summary->set_out(0, Location::Pair(Location::RequiresRegister(),
+                                       Location::RequiresRegister()));
+  } else {
+    ASSERT(representation() == kTagged);
+    summary->set_out(0, Location::RequiresRegister());
+  }
+
+  return summary;
+}
+
+
+void LoadCodeUnitsInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  const Register array = locs()->in(0).reg();
+  const Location index = locs()->in(1);
+
+  Address element_address = __ ElementAddressForRegIndex(
+        true,  IsExternal(), class_id(), index_scale(), array, index.reg());
+  // Warning: element_address may use register IP as base.
+
+  if (representation() == kUnboxedMint) {
+    ASSERT(compiler->is_optimizing());
+    ASSERT(locs()->out(0).IsPairLocation());
+    PairLocation* result_pair = locs()->out(0).AsPairLocation();
+    Register result1 = result_pair->At(0).reg();
+    Register result2 = result_pair->At(1).reg();
+    switch (class_id()) {
+      case kOneByteStringCid:
+      case kExternalOneByteStringCid:
+        ASSERT(element_count() == 4);
+        __ ldr(result1, element_address);
+        __ eor(result2, result2, Operand(result2));
+        break;
+      case kTwoByteStringCid:
+      case kExternalTwoByteStringCid:
+        ASSERT(element_count() == 2);
+        __ ldr(result1, element_address);
+        __ eor(result2, result2, Operand(result2));
+        break;
+      default:
+        UNREACHABLE();
+    }
+  } else {
+    ASSERT(representation() == kTagged);
+    Register result = locs()->out(0).reg();
+    switch (class_id()) {
+      case kOneByteStringCid:
+      case kExternalOneByteStringCid:
+        switch (element_count()) {
+          case 1: __ ldrb(result, element_address); break;
+          case 2: __ ldrh(result, element_address); break;
+          case 4: __ ldr(result, element_address); break;
+          default: UNREACHABLE();
+        }
+        break;
+      case kTwoByteStringCid:
+      case kExternalTwoByteStringCid:
+        switch (element_count()) {
+          case 1: __ ldrh(result, element_address); break;
+          case 2: __ ldr(result, element_address); break;
+          default: UNREACHABLE();
+        }
+        break;
+      default:
+        UNREACHABLE();
+        break;
+    }
+    if (can_pack_into_smi()) {
+      __ SmiTag(result);
+    } else {
+      // If the value cannot fit in a smi then allocate a mint box for it.
+      Register value = locs()->temp(0).reg();
+      Register temp = locs()->temp(1).reg();
+      ASSERT(result != value);
+      __ MoveRegister(value, result);
+      __ SmiTag(result);
+
+      Label done;
+      __ TestImmediate(value, 0xC0000000);
+      __ b(&done, EQ);
+      BoxAllocationSlowPath::Allocate(
+          compiler, this, compiler->mint_class(), result, temp);
+      __ eor(temp, temp, Operand(temp));
+      __ StoreToOffset(kWord, value, result,
+                       Mint::value_offset() - kHeapObjectTag);
+      __ StoreToOffset(kWord, temp, result,
+                       Mint::value_offset() - kHeapObjectTag + kWordSize);
+      __ Bind(&done);
+    }
+  }
+}
+
+
 LocationSummary* StoreInstanceFieldInstr::MakeLocationSummary(Isolate* isolate,
                                                               bool opt) const {
   const intptr_t kNumInputs = 2;