Don't assign safepoint at the definition to the definition's live-range.

This avoids treating spill slot associated with the definition live during GC as
it will only be filled in after definition's code is fully executed.

Additionally this makes output register dead for GC inside the definiton itself
which is something we used to guarantee manually by removing it from the
live_registers set before calling on the slow path.

BUG=
R=fschneider@google.com

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

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

runtime/vm/code_generator.cc

@@ -70,6 +70,11 @@ DEFINE_FLAG(int, deoptimize_every, 0,
 DEFINE_FLAG(charp, deoptimize_filter, NULL,
             "Deoptimize in named function on stack overflow checks");
 
+#ifdef DEBUG
+DEFINE_FLAG(charp, gc_at_instance_allocation, NULL,
+            "Perform a GC before allocation of instances of "
+            "the specified class");
+#endif
 
 DEFINE_RUNTIME_ENTRY(TraceFunctionEntry, 1) {
   const Function& function = Function::CheckedHandle(arguments.ArgAt(0));
@@ -141,7 +146,20 @@ static intptr_t GetCallerLocation() {
 // Return value: newly allocated object.
 DEFINE_RUNTIME_ENTRY(AllocateObject, 2) {
   const Class& cls = Class::CheckedHandle(arguments.ArgAt(0));
+
+#ifdef DEBUG
+  if (FLAG_gc_at_instance_allocation != NULL) {
+    const String& name = String::Handle(cls.Name());
+    if (String::EqualsIgnoringPrivateKey(
+            name,
+            String::Handle(String::New(FLAG_gc_at_instance_allocation)))) {
+      Isolate::Current()->heap()->CollectAllGarbage();
+    }
+  }
+#endif
+
   const Instance& instance = Instance::Handle(Instance::New(cls));
+
   arguments.SetReturn(instance);
   if (cls.NumTypeArguments() == 0) {
     // No type arguments required for a non-parameterized type.

runtime/vm/flow_graph_allocator.cc

@@ -610,7 +610,7 @@ void FlowGraphAllocator::ProcessInitialDefinition(Definition* defn,
     range->set_assigned_location(Location::Constant(constant->value()));
     range->set_spill_slot(Location::Constant(constant->value()));
   }
-  AssignSafepoints(range);
+  AssignSafepoints(defn, range);
   range->finger()->Initialize(range);
   UsePosition* use =
       range->finger()->FirstRegisterBeneficialUse(block->start_pos());
@@ -783,7 +783,7 @@ void FlowGraphAllocator::ConnectIncomingPhiMoves(JoinEntryInstr* join) {
 
     // All phi resolution moves are connected. Phi's live range is
     // complete.
-    AssignSafepoints(range);
+    AssignSafepoints(phi, range);
 
     CompleteRange(range, RegisterKindForResult(phi));
 
@@ -994,7 +994,6 @@ void FlowGraphAllocator::ProcessOneInput(BlockEntryInstr* block,
 
 
 void FlowGraphAllocator::ProcessOneOutput(BlockEntryInstr* block,
-                                          Instruction* current,
                                           intptr_t pos,
                                           Location* out,
                                           Definition* def,
@@ -1103,8 +1102,8 @@ void FlowGraphAllocator::ProcessOneOutput(BlockEntryInstr* block,
     range->AddUse(pos, out);
   }
 
-  AssignSafepoints(range);
-  CompleteRange(range, RegisterKindForResult(current));
+  AssignSafepoints(def, range);
+  CompleteRange(range, RegisterKindForResult(def));
 }
 
 
@@ -1301,14 +1300,14 @@ void FlowGraphAllocator::ProcessOneInstruction(BlockEntryInstr* block,
       ASSERT(input->HasPairRepresentation());
       // Each element of the pair is assigned it's own virtual register number
       // and is allocated its own LiveRange.
-      ProcessOneOutput(block, current, pos,  // BlockEntry, Instruction, seq.
+      ProcessOneOutput(block, pos,  // BlockEntry, seq.
                        pair->SlotAt(0), def,  // (output) Location, Definition.
                        def->ssa_temp_index(),  // (output) virtual register.
                        true,  // output mapped to first input.
                        in_pair->SlotAt(0), input,  // (input) Location, Def.
                        input->ssa_temp_index(),  // (input) virtual register.
                        interference_set);
-      ProcessOneOutput(block, current, pos,
+      ProcessOneOutput(block, pos,
                        pair->SlotAt(1), def,
                        ToSecondPairVreg(def->ssa_temp_index()),
                        true,
@@ -1318,13 +1317,13 @@ void FlowGraphAllocator::ProcessOneInstruction(BlockEntryInstr* block,
     } else {
       // Each element of the pair is assigned it's own virtual register number
       // and is allocated its own LiveRange.
-      ProcessOneOutput(block, current, pos,
+      ProcessOneOutput(block, pos,
                        pair->SlotAt(0), def,
                        def->ssa_temp_index(),
                        false,            // output is not mapped to first input.
                        NULL, NULL, -1,   // First input not needed.
                        interference_set);
-      ProcessOneOutput(block, current, pos,
+      ProcessOneOutput(block, pos,
                        pair->SlotAt(1), def,
                        ToSecondPairVreg(def->ssa_temp_index()),
                        false,
@@ -1336,7 +1335,7 @@ void FlowGraphAllocator::ProcessOneInstruction(BlockEntryInstr* block,
       Location* in_ref = locs->in_slot(0);
       Definition* input = current->InputAt(0)->definition();
       ASSERT(!in_ref->IsPairLocation());
-      ProcessOneOutput(block, current, pos,  // BlockEntry, Instruction, seq.
+      ProcessOneOutput(block, pos,  // BlockEntry, Instruction, seq.
                        out, def,  // (output) Location, Definition.
                        def->ssa_temp_index(),  // (output) virtual register.
                        true,  // output mapped to first input.
@@ -1344,7 +1343,7 @@ void FlowGraphAllocator::ProcessOneInstruction(BlockEntryInstr* block,
                        input->ssa_temp_index(),  // (input) virtual register.
                        interference_set);
     } else {
-      ProcessOneOutput(block, current, pos,
+      ProcessOneOutput(block, pos,
                        out, def,
                        def->ssa_temp_index(),
                        false,            // output is not mapped to first input.
@@ -2463,14 +2462,23 @@ bool LiveRange::Contains(intptr_t pos) const {
 }
 
 
-void FlowGraphAllocator::AssignSafepoints(LiveRange* range) {
+void FlowGraphAllocator::AssignSafepoints(Definition* defn,
+                                          LiveRange* range) {
   for (intptr_t i = safepoints_.length() - 1; i >= 0; i--) {
-    Instruction* instr = safepoints_[i];
+    Instruction* safepoint_instr = safepoints_[i];
+    if (safepoint_instr == defn) {
+      // The value is not live until after the definition is fully executed,
+      // don't assign the safepoint inside the definition itself to
+      // definition's liverange.
+      continue;
+    }
 
-    const intptr_t pos = instr->lifetime_position();
+    const intptr_t pos = safepoint_instr->lifetime_position();
     if (range->End() <= pos) break;
 
-    if (range->Contains(pos)) range->AddSafepoint(pos, instr->locs());
+    if (range->Contains(pos)) {
+      range->AddSafepoint(pos, safepoint_instr->locs());
+    }
   }
 }
 

runtime/vm/flow_graph_allocator.h

@@ -116,7 +116,6 @@ class FlowGraphAllocator : public ValueObject {
                        intptr_t vreg,
                        RegisterSet* live_registers);
   void ProcessOneOutput(BlockEntryInstr* block,
-                        Instruction* current,
                         intptr_t pos,
                         Location* out,
                         Definition* def,
@@ -143,7 +142,7 @@ class FlowGraphAllocator : public ValueObject {
   intptr_t NumberOfRegisters() const { return number_of_registers_; }
 
   // Find all safepoints that are covered by this live range.
-  void AssignSafepoints(LiveRange* range);
+  void AssignSafepoints(Definition* defn, LiveRange* range);
 
   void PrepareForAllocation(Location::Kind register_kind,
                             intptr_t number_of_registers,

runtime/vm/intermediate_language_arm.cc

@@ -1930,7 +1930,7 @@ class StoreInstanceFieldSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    locs->live_registers()->Remove(locs->temp(0));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2408,7 +2408,7 @@ class BoxDoubleSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2443,7 +2443,7 @@ class BoxFloat32x4SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2478,7 +2478,7 @@ class BoxFloat64x2SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -3718,7 +3718,7 @@ class BoxInt32x4SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -5940,7 +5940,7 @@ class BoxIntegerSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.

runtime/vm/intermediate_language_arm64.cc

@@ -1618,7 +1618,7 @@ class StoreInstanceFieldSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    locs->live_registers()->Remove(locs->temp(0));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -1992,7 +1992,7 @@ class BoxDoubleSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2027,7 +2027,7 @@ class BoxFloat32x4SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2062,7 +2062,7 @@ class BoxFloat64x2SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -3221,7 +3221,7 @@ class BoxInt32x4SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.

runtime/vm/intermediate_language_ia32.cc

@@ -1717,7 +1717,8 @@ class StoreInstanceFieldSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    locs->live_registers()->Remove(locs->temp(0));
+
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
                            &label,
@@ -2213,8 +2214,7 @@ class BoxDoubleSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
-
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
                            &label,
@@ -2248,7 +2248,7 @@ class BoxFloat32x4SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2283,7 +2283,7 @@ class BoxFloat64x2SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2586,7 +2586,7 @@ class AllocateContextSlowPath : public SlowPathCode {
     __ Bind(entry_label());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
 
@@ -3631,7 +3631,7 @@ class BoxInt32x4SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -5715,7 +5715,7 @@ class BoxIntegerSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.

runtime/vm/intermediate_language_mips.cc

@@ -1733,7 +1733,7 @@ class StoreInstanceFieldSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    locs->live_registers()->Remove(locs->temp(0));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2142,7 +2142,7 @@ class BoxDoubleSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.

runtime/vm/intermediate_language_x64.cc

@@ -1558,7 +1558,7 @@ class StoreInstanceFieldSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    locs->live_registers()->Remove(locs->temp(0));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2047,7 +2047,7 @@ class BoxDoubleSlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2082,7 +2082,7 @@ class BoxFloat32x4SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -2117,7 +2117,7 @@ class BoxFloat64x2SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.
@@ -3451,7 +3451,7 @@ class BoxInt32x4SlowPath : public SlowPathCode {
     const ExternalLabel label(stub.EntryPoint());
 
     LocationSummary* locs = instruction_->locs();
-    locs->live_registers()->Remove(locs->out(0));
+    ASSERT(!locs->live_registers()->Contains(locs->out(0)));
 
     compiler->SaveLiveRegisters(locs);
     compiler->GenerateCall(Scanner::kNoSourcePos,  // No token position.

runtime/vm/locations.h

@@ -445,6 +445,17 @@ class RegisterSet : public ValueObject {
     }
   }
 
+  bool Contains(Location loc) {
+    if (loc.IsRegister()) {
+      return ContainsRegister(loc.reg());
+    } else if (loc.IsFpuRegister()) {
+      return ContainsFpuRegister(loc.fpu_reg());
+    } else {
+      UNREACHABLE();
+      return false;
+    }
+  }
+
   void DebugPrint() {
     for (intptr_t i = 0; i < kNumberOfCpuRegisters; i++) {
       Register r = static_cast<Register>(i);

tests/standalone/slowpath_safepoints_test.dart

@@ -0,0 +1,49 @@
+// 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 safepoints associated with slowpaths don't mark non-existing values
+// alive.
+// VMOptions=--optimization-counter-threshold=5 --no-inline_alloc --gc_at_instance_allocation=_Double
+
+class C {
+  final next;
+  C(this.next);
+}
+
+
+noop(a1, a2, a3, a4, a5, a6, a7, a8, a9) => 0;
+
+crash(f, i) {
+  final obj1 = new C(null);
+  final obj2 = new C(obj1);
+  final obj3 = new C(obj2);
+  final obj4 = new C(obj3);
+  final obj5 = new C(obj4);
+  final obj6 = new C(obj5);
+  final obj7 = new C(obj6);
+  final obj8 = new C(obj7);
+  final obj9 = new C(obj8);
+
+  f(obj1, obj2, obj3, obj4, obj5, obj6, obj7, obj8, obj9);
+  f(obj1, obj2, obj3, obj4, obj5, obj6, obj7, obj8, obj9);
+
+  final d1 = (i + 0).toDouble();
+  final d2 = (i + 1).toDouble();
+  final d3 = (i + 2).toDouble();
+  final d4 = (i + 3).toDouble();
+  final d5 = (i + 4).toDouble();
+  final d6 = (i + 5).toDouble();
+  final d7 = (i + 6).toDouble();
+  final d8 = (i + 7).toDouble();
+  final d9 = (i + 8).toDouble();
+
+  f(d1, d2, d3, d4, d5, d6, d7, d8, d9);
+  f(d1, d2, d3, d4, d5, d6, d7, d8, d9);
+}
+
+main() {
+  for (var i = 0; i < 10; i++) {
+    print(i);
+    crash(noop, 10);
+  }
+}