[crankshaft] Disable further folding already folded allocations.

bmeurer · Commit bot · commit 7b79224b21a2 · 2016-08-31T09:48:45.000Z
When we try to further fold previously folded allocations in Crankshaft GVN we don't properly transform the allocations involved, which causes the mechanism to leave holes in the new/old space (and thereby violate the iterability property of the new/old space). BUG=chromium:621868 R=jarin@chromium.org Review-Url: https://codereview.chromium.org/2297983003 Cr-Commit-Position: refs/heads/master@{#39040}
diff --git a/src/crankshaft/hydrogen-instructions.cc b/src/crankshaft/hydrogen-instructions.cc
@@ -3184,6 +3184,13 @@ bool HAllocate::HandleSideEffectDominator(GVNFlag side_effect,
     return false;
   }
 
+  if (IsAllocationFoldingDominator()) {
+    if (FLAG_trace_allocation_folding) {
+      PrintF("#%d (%s) cannot fold into #%d (%s), already dominator\n", id(),
+             Mnemonic(), dominator->id(), dominator->Mnemonic());
+    }
+    return false;
+  }
 
   if (!IsFoldable(dominator_allocate)) {
     if (FLAG_trace_allocation_folding) {
@@ -3235,17 +3242,6 @@ bool HAllocate::HandleSideEffectDominator(GVNFlag side_effect,
     }
   }
 
-  if (IsAllocationFoldingDominator()) {
-    DeleteAndReplaceWith(dominator_allocate);
-    if (FLAG_trace_allocation_folding) {
-      PrintF(
-          "#%d (%s) folded dominator into #%d (%s), new dominator size: %d\n",
-          id(), Mnemonic(), dominator_allocate->id(),
-          dominator_allocate->Mnemonic(), new_dominator_size);
-    }
-    return true;
-  }
-
   if (!dominator_allocate->IsAllocationFoldingDominator()) {
     HAllocate* first_alloc =
         HAllocate::New(isolate, zone, dominator_allocate->context(),
@@ -3280,6 +3276,8 @@ std::ostream& HAllocate::PrintDataTo(std::ostream& os) const {  // NOLINT
   if (IsOldSpaceAllocation()) os << "P";
   if (MustAllocateDoubleAligned()) os << "A";
   if (MustPrefillWithFiller()) os << "F";
+  if (IsAllocationFoldingDominator()) os << "d";
+  if (IsAllocationFolded()) os << "f";
   return os << ")";
 }
 
diff --git a/src/crankshaft/hydrogen-instructions.h b/src/crankshaft/hydrogen-instructions.h
@@ -4935,7 +4935,7 @@ class HAllocate final : public HTemplateInstruction<3> {
         static_cast<HAllocate::Flags>(flags_ | ALLOCATION_FOLDING_DOMINATOR);
   }
 
-  bool IsAllocationFoldingDominator() {
+  bool IsAllocationFoldingDominator() const {
     return (flags_ & ALLOCATION_FOLDING_DOMINATOR) != 0;
   }
 
@@ -4946,7 +4946,7 @@ class HAllocate final : public HTemplateInstruction<3> {
     SetOperandAt(2, dominator);
   }
 
-  bool IsAllocationFolded() { return (flags_ & ALLOCATION_FOLDED) != 0; }
+  bool IsAllocationFolded() const { return (flags_ & ALLOCATION_FOLDED) != 0; }
 
   bool HandleSideEffectDominator(GVNFlag side_effect,
                                  HValue* dominator) override;
diff --git a/test/mjsunit/regress/regress-crbug-621868.js b/test/mjsunit/regress/regress-crbug-621868.js
@@ -0,0 +1,20 @@
+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --verify-heap
+
+function f(a) {  // First parameter is tagged.
+  var n = 1 + a;
+}
+
+function g() {
+  f();
+  var d = {x : f()};
+  return [d];
+}
+
+g();
+g();
+%OptimizeFunctionOnNextCall(g);
+g();

Original file line number	Diff line number	Diff line change
`@@ -4935,7 +4935,7 @@ class HAllocate final : public HTemplateInstruction<3> {`
`4935`	`4935`	`static_cast<HAllocate::Flags>(flags_ \| ALLOCATION_FOLDING_DOMINATOR);`
`4936`	`4936`	`}`
`4937`	`4937`
`4938`		`- bool IsAllocationFoldingDominator() {`
	`4938`	`+ bool IsAllocationFoldingDominator() const {`
`4939`	`4939`	`return (flags_ & ALLOCATION_FOLDING_DOMINATOR) != 0;`
`4940`	`4940`	`}`
`4941`	`4941`
`@@ -4946,7 +4946,7 @@ class HAllocate final : public HTemplateInstruction<3> {`
`4946`	`4946`	`SetOperandAt(2, dominator);`
`4947`	`4947`	`}`
`4948`	`4948`
`4949`		`- bool IsAllocationFolded() { return (flags_ & ALLOCATION_FOLDED) != 0; }`
	`4949`	`+ bool IsAllocationFolded() const { return (flags_ & ALLOCATION_FOLDED) != 0; }`
`4950`	`4950`
`4951`	`4951`	`bool HandleSideEffectDominator(GVNFlag side_effect,`
`4952`	`4952`	`HValue* dominator) override;`