Support remaping of pair location inside materializations for slow paths.

Move remaping logic into Location::RemapForSlowPath to ensure that it is shared and will not get out of sync again.

BUG=http://dartbug.com/21887
R=fschneider@google.com

Review URL: https://codereview.chromium.org//811823002

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@42396 260f80e4-7a28-3924-810f-c04153c831b5

runtime/vm/flow_graph_compiler.cc

@@ -688,57 +688,11 @@ Environment* FlowGraphCompiler::SlowPathEnvironmentFor(
   //    corresponding spill slot locations.
   for (Environment::DeepIterator it(env); !it.Done(); it.Advance()) {
     Location loc = it.CurrentLocation();
-    if (loc.IsRegister()) {
-      intptr_t index = cpu_reg_slots[loc.reg()];
-      ASSERT(index >= 0);
-      it.SetCurrentLocation(Location::StackSlot(index));
-    } else if (loc.IsFpuRegister()) {
-      intptr_t index = fpu_reg_slots[loc.fpu_reg()];
-      ASSERT(index >= 0);
-      Value* value = it.CurrentValue();
-      switch (value->definition()->representation()) {
-        case kUnboxedDouble:
-          it.SetCurrentLocation(Location::DoubleStackSlot(index));
-          break;
-        case kUnboxedFloat32x4:
-        case kUnboxedInt32x4:
-        case kUnboxedFloat64x2:
-          it.SetCurrentLocation(Location::QuadStackSlot(index));
-          break;
-        default:
-          UNREACHABLE();
-      }
-    } else if (loc.IsPairLocation()) {
-      intptr_t representation =
-          it.CurrentValue()->definition()->representation();
-      ASSERT(representation == kUnboxedMint);
-      PairLocation* value_pair = loc.AsPairLocation();
-      intptr_t index_lo;
-      intptr_t index_hi;
-      if (value_pair->At(0).IsRegister()) {
-        index_lo = cpu_reg_slots[value_pair->At(0).reg()];
-      } else {
-        ASSERT(value_pair->At(0).IsStackSlot());
-        index_lo = value_pair->At(0).stack_index();
-      }
-      if (value_pair->At(1).IsRegister()) {
-        index_hi = cpu_reg_slots[value_pair->At(1).reg()];
-      } else {
-        ASSERT(value_pair->At(1).IsStackSlot());
-        index_hi = value_pair->At(1).stack_index();
-      }
-      it.SetCurrentLocation(Location::Pair(Location::StackSlot(index_lo),
-                                           Location::StackSlot(index_hi)));
-    } else if (loc.IsInvalid()) {
-      Definition* def =
-          it.CurrentValue()->definition();
-      ASSERT(def != NULL);
-      if (def->IsMaterializeObject()) {
-        def->AsMaterializeObject()->RemapRegisters(fpu_reg_slots,
-                                                   cpu_reg_slots);
-      }
-    }
+    Value* value = it.CurrentValue();
+    it.SetCurrentLocation(loc.RemapForSlowPath(
+        value->definition(), cpu_reg_slots, fpu_reg_slots));
   }
+
   return env;
 }
 

runtime/vm/intermediate_language.cc

@@ -2704,42 +2704,16 @@ void MaterializeObjectInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
 
 // This function should be kept in sync with
 // FlowGraphCompiler::SlowPathEnvironmentFor().
-void MaterializeObjectInstr::RemapRegisters(intptr_t* fpu_reg_slots,
-                                            intptr_t* cpu_reg_slots) {
+void MaterializeObjectInstr::RemapRegisters(intptr_t* cpu_reg_slots,
+                                            intptr_t* fpu_reg_slots) {
   if (registers_remapped_) {
     return;
   }
   registers_remapped_ = true;
 
   for (intptr_t i = 0; i < InputCount(); i++) {
-    Location loc = LocationAt(i);
-    if (loc.IsRegister()) {
-      intptr_t index = cpu_reg_slots[loc.reg()];
-      ASSERT(index >= 0);
-      locations_[i] = Location::StackSlot(index);
-    } else if (loc.IsFpuRegister()) {
-      intptr_t index = fpu_reg_slots[loc.fpu_reg()];
-      ASSERT(index >= 0);
-      Value* value = InputAt(i);
-      switch (value->definition()->representation()) {
-        case kUnboxedDouble:
-          locations_[i] = Location::DoubleStackSlot(index);
-          break;
-        case kUnboxedFloat32x4:
-        case kUnboxedInt32x4:
-        case kUnboxedFloat64x2:
-          locations_[i] = Location::QuadStackSlot(index);
-          break;
-        default:
-          UNREACHABLE();
-      }
-    } else if (loc.IsPairLocation()) {
-      UNREACHABLE();
-    } else if (loc.IsInvalid() &&
-               InputAt(i)->definition()->IsMaterializeObject()) {
-      InputAt(i)->definition()->AsMaterializeObject()->RemapRegisters(
-          fpu_reg_slots, cpu_reg_slots);
-    }
+    locations_[i] = LocationAt(i).RemapForSlowPath(
+        InputAt(i)->definition(), cpu_reg_slots, fpu_reg_slots);
   }
 }
 

runtime/vm/intermediate_language.h

@@ -4142,8 +4142,8 @@ class MaterializeObjectInstr : public Definition {
 
   virtual bool MayThrow() const { return false; }
 
-  void RemapRegisters(intptr_t* fpu_reg_slots,
-                      intptr_t* cpu_reg_slots);
+  void RemapRegisters(intptr_t* cpu_reg_slots,
+                      intptr_t* fpu_reg_slots);
 
   bool was_visited_for_liveness() const { return visited_for_liveness_; }
   void mark_visited_for_liveness() {

runtime/vm/locations.cc

@@ -244,6 +244,59 @@ Location Location::Copy() const {
 }
 
 
+Location Location::RemapForSlowPath(Definition* def,
+                                    intptr_t* cpu_reg_slots,
+                                    intptr_t* fpu_reg_slots) const {
+  if (IsRegister()) {
+    intptr_t index = cpu_reg_slots[reg()];
+    ASSERT(index >= 0);
+    return Location::StackSlot(index);
+  } else if (IsFpuRegister()) {
+    intptr_t index = fpu_reg_slots[fpu_reg()];
+    ASSERT(index >= 0);
+    switch (def->representation()) {
+      case kUnboxedDouble:
+        return Location::DoubleStackSlot(index);
+
+      case kUnboxedFloat32x4:
+      case kUnboxedInt32x4:
+      case kUnboxedFloat64x2:
+        return Location::QuadStackSlot(index);
+
+      default:
+        UNREACHABLE();
+    }
+  } else if (IsPairLocation()) {
+    ASSERT(def->representation() == kUnboxedMint);
+    PairLocation* value_pair = AsPairLocation();
+    intptr_t index_lo;
+    intptr_t index_hi;
+
+    if (value_pair->At(0).IsRegister()) {
+      index_lo = cpu_reg_slots[value_pair->At(0).reg()];
+    } else {
+      ASSERT(value_pair->At(0).IsStackSlot());
+      index_lo = value_pair->At(0).stack_index();
+    }
+
+    if (value_pair->At(1).IsRegister()) {
+      index_hi = cpu_reg_slots[value_pair->At(1).reg()];
+    } else {
+      ASSERT(value_pair->At(1).IsStackSlot());
+      index_hi = value_pair->At(1).stack_index();
+    }
+
+    return Location::Pair(Location::StackSlot(index_lo),
+                          Location::StackSlot(index_hi));
+  } else if (IsInvalid() && def->IsMaterializeObject()) {
+    def->AsMaterializeObject()->RemapRegisters(cpu_reg_slots, fpu_reg_slots);
+    return *this;
+  }
+
+  return *this;
+}
+
+
 void LocationSummary::PrintTo(BufferFormatter* f) const {
   if (input_count() > 0) {
     f->Print(" (");

runtime/vm/locations.h

@@ -11,10 +11,12 @@
 
 namespace dart {
 
+
 class BufferFormatter;
-class Value;
-class PairLocation;
 class ConstantInstr;
+class Definition;
+class PairLocation;
+class Value;
 
 
 enum Representation {
@@ -370,6 +372,10 @@ class Location : public ValueObject {
 
   Location Copy() const;
 
+  Location RemapForSlowPath(Definition* def,
+                            intptr_t* cpu_reg_slots,
+                            intptr_t* fpu_reg_slots) const;
+
  private:
   explicit Location(uword value) : value_(value) { }
 

tests/standalone/pair_location_remapping_test.dart

@@ -0,0 +1,30 @@
+// Copyright (c) 2014, 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.
+// Test that pair locations are remaped in slow path environments.
+// VMOptions=--optimization_counter_threshold=10 --no-use-osr
+
+import "package:expect/expect.dart";
+
+class A {
+  final f;
+  A(this.f);
+}
+
+foo(i) {
+  var j = 0x7fffffffffffffff + i;
+  var c = new A(j);  // allocation will be sunk
+  var r = 0;
+  for (var k = 0; k < 10; k++) {
+    if ((j & (1 << k)) != 0) {
+      r++;
+    }
+  }
+  return c.f - r;
+}
+
+main() {
+  for (var i = 0; i < 1000; i++) {
+    Expect.equals(0x7fffffffffffffff - 10, foo(0));
+  }
+}

tests/standalone/standalone.status

@@ -88,6 +88,7 @@ issue14236_test: Skip # dart2js does not deal with Script snapshots.
 javascript_compatibility_errors_test: Skip
 javascript_compatibility_warnings_test: Skip
 unboxed_int_converter_test: Skip
+pair_location_remapping_test: Skip
 
 [ $compiler == dart2js && $jscl ]
 assert_test: RuntimeError, OK # Assumes unspecified fields on the AssertionError.