From 56b6b6a8fa8e06b729c6e2e9a12524d9c95d85a5 Mon Sep 17 00:00:00 2001 From: Jaroslav Sevcik Date: Mon, 1 Oct 2018 07:30:37 +0200 Subject: [PATCH] [turbofan] Make sure we use only serialized elements kind transitions. Currently, we call the MapRef::AsElementsKind method on an initial map multiple times (from JSCreateLowering::ReduceJSCreateArray). However, this does not does not play well with the heap copier/broker, which only expectes AsElementsKind to be called on initial maps. This CL makes sure we only call AsElementsKind once (on the initial map). Bug: chromium:890620 Change-Id: If44421d3900abb7629ea8f789a005b8d8ebaf881 Reviewed-on: https://chromium-review.googlesource.com/1253105 Reviewed-by: Georg Neis Commit-Queue: Jaroslav Sevcik Cr-Commit-Position: refs/heads/master@{#56307} --- src/compiler/js-create-lowering.cc | 50 ++++++++++++------------- src/compiler/js-create-lowering.h | 7 ++-- test/mjsunit/compiler/regress-890620.js | 25 +++++++++++++ 3 files changed, 53 insertions(+), 29 deletions(-) create mode 100644 test/mjsunit/compiler/regress-890620.js diff --git a/src/compiler/js-create-lowering.cc b/src/compiler/js-create-lowering.cc index 268d52c790..c5801e4b5c 100644 --- a/src/compiler/js-create-lowering.cc +++ b/src/compiler/js-create-lowering.cc @@ -475,7 +475,8 @@ Reduction JSCreateLowering::ReduceJSCreateGeneratorObject(Node* node) { // Constructs an array with a variable {length} when no upper bound // is known for the capacity. Reduction JSCreateLowering::ReduceNewArray( - Node* node, Node* length, MapRef initial_map, PretenureFlag pretenure, + Node* node, Node* length, MapRef initial_map, ElementsKind elements_kind, + PretenureFlag pretenure, const SlackTrackingPrediction& slack_tracking_prediction) { DCHECK_EQ(IrOpcode::kJSCreateArray, node->opcode()); Node* effect = NodeProperties::GetEffectInput(node); @@ -484,8 +485,8 @@ Reduction JSCreateLowering::ReduceNewArray( // Constructing an Array via new Array(N) where N is an unsigned // integer, always creates a holey backing store. ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING( - initial_map, initial_map.AsElementsKind( - GetHoleyElementsKind(initial_map.elements_kind()))); + initial_map, + initial_map.AsElementsKind(GetHoleyElementsKind(elements_kind))); // Check that the {limit} is an unsigned integer in the valid range. // This has to be kept in sync with src/runtime/runtime-array.cc, @@ -524,7 +525,7 @@ Reduction JSCreateLowering::ReduceNewArray( // upper bound is known for the {capacity}. Reduction JSCreateLowering::ReduceNewArray( Node* node, Node* length, int capacity, MapRef initial_map, - PretenureFlag pretenure, + ElementsKind elements_kind, PretenureFlag pretenure, const SlackTrackingPrediction& slack_tracking_prediction) { DCHECK(node->opcode() == IrOpcode::kJSCreateArray || node->opcode() == IrOpcode::kJSCreateEmptyLiteralArray); @@ -532,12 +533,11 @@ Reduction JSCreateLowering::ReduceNewArray( Node* control = NodeProperties::GetControlInput(node); // Determine the appropriate elements kind. - ElementsKind elements_kind = initial_map.elements_kind(); if (NodeProperties::GetType(length).Max() > 0.0) { elements_kind = GetHoleyElementsKind(elements_kind); - ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING( - initial_map, initial_map.AsElementsKind(elements_kind)); } + ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING( + initial_map, initial_map.AsElementsKind(elements_kind)); DCHECK(IsFastElementsKind(elements_kind)); // Setup elements and properties. @@ -569,15 +569,16 @@ Reduction JSCreateLowering::ReduceNewArray( Reduction JSCreateLowering::ReduceNewArray( Node* node, std::vector values, MapRef initial_map, - PretenureFlag pretenure, + ElementsKind elements_kind, PretenureFlag pretenure, const SlackTrackingPrediction& slack_tracking_prediction) { DCHECK_EQ(IrOpcode::kJSCreateArray, node->opcode()); Node* effect = NodeProperties::GetEffectInput(node); Node* control = NodeProperties::GetControlInput(node); // Determine the appropriate elements kind. - ElementsKind elements_kind = initial_map.elements_kind(); DCHECK(IsFastElementsKind(elements_kind)); + ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING( + initial_map, initial_map.AsElementsKind(elements_kind)); // Check {values} based on the {elements_kind}. These checks are guarded // by the {elements_kind} feedback on the {site}, so it's safe to just @@ -663,10 +664,9 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) { bool can_inline_call = false; // Check if we have a feedback {site} on the {node}. + ElementsKind elements_kind = initial_map.elements_kind(); if (site_ref) { - ElementsKind elements_kind = site_ref->GetElementsKind(); - ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING( - initial_map, initial_map.AsElementsKind(elements_kind)); + elements_kind = site_ref->GetElementsKind(); can_inline_call = site_ref->CanInlineCall(); pretenure = dependencies()->DependOnPretenureMode(*site_ref); dependencies()->DependOnElementsKind(*site_ref); @@ -677,7 +677,8 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) { if (arity == 0) { Node* length = jsgraph()->ZeroConstant(); int capacity = JSArray::kPreallocatedArrayElements; - return ReduceNewArray(node, length, capacity, initial_map, pretenure, + return ReduceNewArray(node, length, capacity, initial_map, + elements_kind, pretenure, slack_tracking_prediction); } else if (arity == 1) { Node* length = NodeProperties::GetValueInput(node, 2); @@ -685,26 +686,25 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) { if (!length_type.Maybe(Type::Number())) { // Handle the single argument case, where we know that the value // cannot be a valid Array length. - ElementsKind elements_kind = initial_map.elements_kind(); elements_kind = GetMoreGeneralElementsKind( elements_kind, IsHoleyElementsKind(elements_kind) ? HOLEY_ELEMENTS : PACKED_ELEMENTS); - ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING( - initial_map, initial_map.AsElementsKind(elements_kind)); return ReduceNewArray(node, std::vector{length}, initial_map, - pretenure, slack_tracking_prediction); + elements_kind, pretenure, + slack_tracking_prediction); } if (length_type.Is(Type::SignedSmall()) && length_type.Min() >= 0 && length_type.Max() <= kElementLoopUnrollLimit && length_type.Min() == length_type.Max()) { int capacity = static_cast(length_type.Max()); - return ReduceNewArray(node, length, capacity, initial_map, pretenure, + return ReduceNewArray(node, length, capacity, initial_map, + elements_kind, pretenure, slack_tracking_prediction); } if (length_type.Maybe(Type::UnsignedSmall()) && can_inline_call) { - return ReduceNewArray(node, length, initial_map, pretenure, - slack_tracking_prediction); + return ReduceNewArray(node, length, initial_map, elements_kind, + pretenure, slack_tracking_prediction); } } else if (arity <= JSArray::kInitialMaxFastElementArray) { // Gather the values to store into the newly created array. @@ -728,7 +728,6 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) { } // Try to figure out the ideal elements kind statically. - ElementsKind elements_kind = initial_map.elements_kind(); if (values_all_smis) { // Smis can be stored with any elements kind. } else if (values_all_numbers) { @@ -749,10 +748,8 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) { // we cannot inline this invocation of the Array constructor here. return NoChange(); } - ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING( - initial_map, initial_map.AsElementsKind(elements_kind)); - return ReduceNewArray(node, values, initial_map, pretenure, - slack_tracking_prediction); + return ReduceNewArray(node, values, initial_map, elements_kind, + pretenure, slack_tracking_prediction); } } } @@ -1097,7 +1094,8 @@ Reduction JSCreateLowering::ReduceJSCreateEmptyLiteralArray(Node* node) { DCHECK(!initial_map.IsInobjectSlackTrackingInProgress()); SlackTrackingPrediction slack_tracking_prediction( initial_map, initial_map.instance_size()); - return ReduceNewArray(node, length, 0, initial_map, pretenure, + return ReduceNewArray(node, length, 0, initial_map, + initial_map.elements_kind(), pretenure, slack_tracking_prediction); } return NoChange(); diff --git a/src/compiler/js-create-lowering.h b/src/compiler/js-create-lowering.h index 1a28400762..4099edb7b6 100644 --- a/src/compiler/js-create-lowering.h +++ b/src/compiler/js-create-lowering.h @@ -67,15 +67,16 @@ class V8_EXPORT_PRIVATE JSCreateLowering final Reduction ReduceJSCreateBlockContext(Node* node); Reduction ReduceJSCreateGeneratorObject(Node* node); Reduction ReduceNewArray( - Node* node, Node* length, MapRef initial_map, PretenureFlag pretenure, + Node* node, Node* length, MapRef initial_map, ElementsKind elements_kind, + PretenureFlag pretenure, const SlackTrackingPrediction& slack_tracking_prediction); Reduction ReduceNewArray( Node* node, Node* length, int capacity, MapRef initial_map, - PretenureFlag pretenure, + ElementsKind elements_kind, PretenureFlag pretenure, const SlackTrackingPrediction& slack_tracking_prediction); Reduction ReduceNewArray( Node* node, std::vector values, MapRef initial_map, - PretenureFlag pretenure, + ElementsKind elements_kind, PretenureFlag pretenure, const SlackTrackingPrediction& slack_tracking_prediction); Reduction ReduceJSCreateObject(Node* node); diff --git a/test/mjsunit/compiler/regress-890620.js b/test/mjsunit/compiler/regress-890620.js new file mode 100644 index 0000000000..f5fc7f4f65 --- /dev/null +++ b/test/mjsunit/compiler/regress-890620.js @@ -0,0 +1,25 @@ +// Copyright 2018 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 + +var a = 42; + +function g(n) { + while (n > 0) { + a = new Array(n); + n--; + } +} + +g(1); + +function f() { + g(); +} + +f(); +%OptimizeFunctionOnNextCall(f); +f(); +assertEquals(1, a.length);