Fix StoreIndexedInstr input representation requirements for Int32/Uint32 arrays.

runtime/vm/intermediate_language_x64.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/globals.h"  // Needed here to get TARGET_ARCH_X64.
 #if defined(TARGET_ARCH_X64)
 
 #include "vm/intermediate_language.h"
 
 #include "vm/dart_entry.h"
@@ skipping 229 matching lines @@
   }
 }
 
 
 LocationSummary* UnboxedConstantInstr::MakeLocationSummary(Isolate* isolate,
                                                            bool opt) const {
   const intptr_t kNumInputs = 0;
   const intptr_t kNumTemps = 0;
   LocationSummary* locs = new(isolate) LocationSummary(
       isolate, kNumInputs, kNumTemps, LocationSummary::kNoCall);
-  locs->set_out(0, Location::RequiresFpuRegister());
+  switch (representation()) {
+    case kUnboxedDouble:
+      locs->set_out(0, Location::RequiresFpuRegister());
+      break;
+    case kUnboxedInt32:
+      locs->set_out(0, Location::RequiresRegister());
+      break;
+    default:
+      UNREACHABLE();
+      break;
+  }
   return locs;
 }
 
 
 void UnboxedConstantInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
-  ASSERT(representation_ == kUnboxedDouble);
   // The register allocator drops constant definitions that have no uses.
   if (!locs()->out(0).IsInvalid()) {
-    XmmRegister result = locs()->out(0).fpu_reg();
-    if (Utils::DoublesBitEqual(Double::Cast(value()).value(), 0.0)) {
-      __ xorps(result, result);
-    } else {
-      __ LoadObject(TMP, value(), PP);
-      __ movsd(result, FieldAddress(TMP, Double::value_offset()));
+    switch (representation()) {
+      case kUnboxedDouble: {
+        XmmRegister result = locs()->out(0).fpu_reg();
+        if (Utils::DoublesBitEqual(Double::Cast(value()).value(), 0.0)) {
+          __ xorps(result, result);
+        } else {
+          __ LoadObject(TMP, value(), PP);
+          __ movsd(result, FieldAddress(TMP, Double::value_offset()));
+        }
+        break;
+      }
+      case kUnboxedInt32:
+        __ movl(locs()->out(0).reg(),
+                Immediate(static_cast<int32_t>(Smi::Cast(value()).Value())));
+        break;
+      default:
+        UNREACHABLE();
     }
   }
 }
 
 
 LocationSummary* AssertAssignableInstr::MakeLocationSummary(Isolate* isolate,
                                                             bool opt) const {
   const intptr_t kNumInputs = 3;
   const intptr_t kNumTemps = 0;
   LocationSummary* summary = new(isolate) LocationSummary(
@@ skipping 800 matching lines @@
   switch (class_id_) {
     case kArrayCid:
     case kOneByteStringCid:
     case kTypedDataInt8ArrayCid:
     case kTypedDataUint8ArrayCid:
     case kExternalTypedDataUint8ArrayCid:
     case kTypedDataUint8ClampedArrayCid:
     case kExternalTypedDataUint8ClampedArrayCid:
     case kTypedDataInt16ArrayCid:
     case kTypedDataUint16ArrayCid:
+      return kTagged;
     case kTypedDataInt32ArrayCid:
+      return kUnboxedInt32;
     case kTypedDataUint32ArrayCid:
-      return kTagged;
+      return kUnboxedUint32;
     case kTypedDataFloat32ArrayCid:
     case kTypedDataFloat64ArrayCid:
       return kUnboxedDouble;
     case kTypedDataFloat32x4ArrayCid:
       return kUnboxedFloat32x4;
     case kTypedDataInt32x4ArrayCid:
       return kUnboxedInt32x4;
     case kTypedDataFloat64x2ArrayCid:
       return kUnboxedFloat64x2;
     default:
@@ skipping 137 matching lines @@
     case kTypedDataInt16ArrayCid:
     case kTypedDataUint16ArrayCid: {
       Register value = locs()->in(2).reg();
       __ SmiUntag(value);
       __ movw(element_address, value);
       break;
     }
     case kTypedDataInt32ArrayCid:
     case kTypedDataUint32ArrayCid: {
       Register value = locs()->in(2).reg();
-      __ SmiUntag(value);
       __ movl(element_address, value);
         break;
     }
     case kTypedDataFloat32ArrayCid:
       __ movss(element_address, locs()->in(2).fpu_reg());
       break;
     case kTypedDataFloat64ArrayCid:
       __ movsd(element_address, locs()->in(2).fpu_reg());
       break;
     case kTypedDataInt32x4ArrayCid:
@@ skipping 4257 matching lines @@
 
 
 CompileType UnaryUint32OpInstr::ComputeType() const {
   return CompileType::FromCid(kSmiCid);
 }
 
 
 DEFINE_UNIMPLEMENTED_INSTRUCTION(BinaryUint32OpInstr)
 DEFINE_UNIMPLEMENTED_INSTRUCTION(ShiftUint32OpInstr)
 DEFINE_UNIMPLEMENTED_INSTRUCTION(UnaryUint32OpInstr)
-DEFINE_UNIMPLEMENTED_INSTRUCTION(BoxInt32Instr)
-DEFINE_UNIMPLEMENTED_INSTRUCTION(UnboxInt32Instr)
 DEFINE_UNIMPLEMENTED_INSTRUCTION(BinaryInt32OpInstr)
-DEFINE_UNIMPLEMENTED_INSTRUCTION(BoxUint32Instr)
-DEFINE_UNIMPLEMENTED_INSTRUCTION(UnboxedIntConverterInstr)
 
 
-LocationSummary* UnboxUint32Instr::MakeLocationSummary(Isolate* isolate,
-                                                       bool opt) const {
+LocationSummary* UnboxIntNInstr::MakeLocationSummary(Isolate* isolate,
+                                                     bool opt) const {
+  const intptr_t kNumInputs = 1;
+  const intptr_t kNumTemps = (!is_truncating() && CanDeoptimize()) ? 1 : 0;
+  LocationSummary* summary = new(isolate) LocationSummary(
+      isolate, kNumInputs, kNumTemps, LocationSummary::kNoCall);
+  summary->set_in(0, Location::RequiresRegister());
+  summary->set_out(0, Location::SameAsFirstInput());
+  if (kNumTemps > 0) {
+    summary->set_temp(0, Location::RequiresRegister());
+  }
+  return summary;
+}
+
+
+void UnboxIntNInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  const intptr_t value_cid = value()->Type()->ToCid();
+  const Register value = locs()->in(0).reg();
+  Label* deopt = CanDeoptimize() ?
+      compiler->AddDeoptStub(deopt_id_, ICData::kDeoptUnboxInteger) : NULL;
+  ASSERT(value == locs()->out(0).reg());
+
+  if (value_cid == kSmiCid) {
+    __ SmiUntag(value);
+  } else if (value_cid == kMintCid) {
+    __ movq(value, FieldAddress(value, Mint::value_offset()));
+  } else {
+    Label done;
+    // Optimistically untag value.
+    __ SmiUntagOrCheckClass(value, kMintCid, &done);
+    __ j(NOT_EQUAL, deopt);
+    // Undo untagging by multiplying value with 2.
+    __ movq(value, Address(value, TIMES_2, Mint::value_offset()));
+    __ Bind(&done);
+  }
+
+  if (!is_truncating() && (deopt != NULL)) {

[zra, 2014/09/10 23:17:22]

Maybe copy your comment from arm64 here as well?

[Vyacheslav Egorov (Google), 2014/09/11 11:49:17]

Done.

+    ASSERT(representation() == kUnboxedInt32);
+    Register temp = locs()->temp(0).reg();
+    __ movsxd(temp, value);
+    __ cmpq(temp, value);
+    __ j(NOT_EQUAL, deopt);
+  }
+}
+
+
+LocationSummary* BoxIntNInstr::MakeLocationSummary(Isolate* isolate,
+                                                   bool opt) const {
+  ASSERT((from_representation() == kUnboxedInt32) ||
+         (from_representation() == kUnboxedUint32));
+  const intptr_t kNumInputs = 1;
+  const intptr_t kNumTemps = 0;
+  LocationSummary* summary = new(isolate) LocationSummary(
+      isolate,
+      kNumInputs,
+      kNumTemps,
+      LocationSummary::kNoCall);
+  summary->set_in(0, Location::RequiresRegister());
+  summary->set_out(0, Location::RequiresRegister());
+  return summary;
+}
+
+
+void BoxIntNInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  const Register value = locs()->in(0).reg();
+  const Register out = locs()->out(0).reg();
+  ASSERT(value != out);
+
+  ASSERT(kSmiTagSize == 1);
+  if (from_representation() == kUnboxedInt32) {
+    __ movsxd(out, value);
+  } else {
+    ASSERT(from_representation() == kUnboxedUint32);
+    __ movl(out, value);
+  }
+  __ SmiTag(out);
+}
+
+
+LocationSummary* UnboxedIntConverterInstr::MakeLocationSummary(Isolate* isolate,
+                                                               bool opt) const {
   const intptr_t kNumInputs = 1;
   const intptr_t kNumTemps = 0;
   LocationSummary* summary = new(isolate) LocationSummary(
       isolate, kNumInputs, kNumTemps, LocationSummary::kNoCall);
-  summary->set_in(0, Location::RequiresRegister());
-  summary->set_out(0, Location::SameAsFirstInput());
+  if (from() == kUnboxedMint) {
+    UNREACHABLE();
+  } else if (to() == kUnboxedMint) {
+    UNREACHABLE();
+  } else {
+    ASSERT((to() == kUnboxedUint32) || (to() == kUnboxedInt32));
+    ASSERT((from() == kUnboxedUint32) || (from() == kUnboxedInt32));
+    summary->set_in(0, Location::RequiresRegister());
+    summary->set_out(0, Location::SameAsFirstInput());
+  }
   return summary;
 }
 
 
-void UnboxUint32Instr::EmitNativeCode(FlowGraphCompiler* compiler) {
-  const intptr_t value_cid = value()->Type()->ToCid();
-  const Register value = locs()->in(0).reg();
-  ASSERT(value == locs()->out(0).reg());
-
-  if (value_cid == kSmiCid) {
-    __ SmiUntag(value);
+void UnboxedIntConverterInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  if (from() == kUnboxedInt32 && to() == kUnboxedUint32) {
+    const Register value = locs()->in(0).reg();
+    const Register out = locs()->out(0).reg();
+    // Representations are bitwise equivalent but we want to normalize
+    // upperbits for safety reasons.
+    // TODO(vegorov) if we ensure that we never use upperbits we could
+    // avoid this.
+    __ movl(out, value);
+  } else if (from() == kUnboxedUint32 && to() == kUnboxedInt32) {
+    // Representations are bitwise equivalent.
+    const Register value = locs()->in(0).reg();
+    const Register out = locs()->out(0).reg();
+    __ movsxd(out, value);
+    if (CanDeoptimize()) {
+      Label* deopt =
+          compiler->AddDeoptStub(deopt_id(), ICData::kDeoptUnboxInteger);
+      __ testl(out, out);
+      __ j(NEGATIVE, deopt);
+    }
+  } else if (from() == kUnboxedMint) {
+    UNREACHABLE();
+  } else if (to() == kUnboxedMint) {
+    ASSERT((from() == kUnboxedUint32) || (from() == kUnboxedInt32));
+    UNREACHABLE();
   } else {
-    Label* deopt = compiler->AddDeoptStub(deopt_id_,
-                                          ICData::kDeoptUnboxInteger);
-    __ testq(value, Immediate(kSmiTagMask));
-    __ j(NOT_ZERO, deopt);
-    __ SmiUntag(value);
+    UNREACHABLE();
   }
 }
 
 
 LocationSummary* ThrowInstr::MakeLocationSummary(Isolate* isolate,
                                                  bool opt) const {
   return new(isolate) LocationSummary(isolate, 0, 0, LocationSummary::kCall);
 }
 
 
@@ skipping 279 matching lines @@
   __ movq(R10, Immediate(kInvalidObjectPointer));
   __ movq(RBX, Immediate(kInvalidObjectPointer));
 #endif
 }
 
 }  // namespace dart
 
 #undef __
 
 #endif  // defined TARGET_ARCH_X64