Reviewers: Benedikt Meurer,
Message:
Could you take a look, please?
Description:
[turbofan] Support lazy deopt for truncating store to a typed array.
The change introduces a second frame state (for the state before
the operation) for the StoreProperty nodes. If the store writes
into a typed array, the frame state is used for lazy deopt from
the to-number conversion that is performed by the store.
BUG=v8:3963
LOG=n
R=bmeu...@chromium.org
Please review this at https://codereview.chromium.org/997983004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+84, -14 lines):
M src/compiler/ast-graph-builder.cc
M src/compiler/js-typed-lowering.cc
M src/compiler/operator-properties.cc
A test/mjsunit/compiler/truncating-store-deopt.js
M test/unittests/compiler/js-operator-unittest.cc
M test/unittests/compiler/js-typed-lowering-unittest.cc
Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc
b/src/compiler/ast-graph-builder.cc
index
63696e45cd2e7f2dbe932a2ac415e1c76e30053e..b5a89dac947678a997380821fd959c11415f6482
100644
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -1737,10 +1737,14 @@ void
AstGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) {
if (CompileTimeValue::IsCompileTimeValue(subexpr)) continue;
VisitForValue(subexpr);
+ Node* frame_state_before = environment()->Checkpoint(
+ subexpr->id(), OutputFrameStateCombine::PokeAt(0));
Node* value = environment()->Pop();
Node* index = jsgraph()->Constant(i);
Node* store = BuildKeyedStore(literal, index, value);
- PrepareFrameState(store, expr->GetIdForElement(i));
+ PrepareFrameStateAfterAndBefore(store, expr->GetIdForElement(i),
+ OutputFrameStateCombine::Ignore(),
+ frame_state_before);
}
environment()->Pop(); // Array literal index.
@@ -1781,7 +1785,10 @@ void
AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value,
Node* object = environment()->Pop();
value = environment()->Pop();
Node* store = BuildKeyedStore(object, key, value);
- PrepareFrameState(store, bailout_id);
+ // TODO(jarin) Provide a real frame state before.
+ PrepareFrameStateAfterAndBefore(store, bailout_id,
+ OutputFrameStateCombine::Ignore(),
+ jsgraph()->EmptyFrameState());
break;
}
}
@@ -1812,6 +1819,8 @@ void AstGraphBuilder::VisitAssignment(Assignment*
expr) {
// Evaluate the value and potentially handle compound assignments by
loading
// the left-hand side value and performing a binary operation.
+ Node* frame_state_before_store = nullptr;
+ bool needs_frame_state_before = (assign_type == KEYED_PROPERTY);
if (expr->is_compound()) {
Node* old_value = NULL;
switch (assign_type) {
@@ -1853,8 +1862,21 @@ void AstGraphBuilder::VisitAssignment(Assignment*
expr) {
OutputFrameStateCombine::Push(),
frame_state_before);
environment()->Push(value);
+ if (needs_frame_state_before) {
+ frame_state_before_store = environment()->Checkpoint(
+ expr->binary_operation()->id(),
OutputFrameStateCombine::PokeAt(0));
+ }
} else {
VisitForValue(expr->value());
+ if (needs_frame_state_before) {
+ // This frame state can be used for lazy-deopting from a to-number
+ // conversion if we are storing into a typed array. It is important
+ // that the frame state is usable for such lazy deopt (i.e., it has
+ // to specify how to override the value before the conversion, in
this
+ // case, it overwrites the stack top).
+ frame_state_before_store = environment()->Checkpoint(
+ expr->value()->id(), OutputFrameStateCombine::PokeAt(0));
+ }
}
// Store the value.
@@ -1877,7 +1899,9 @@ void AstGraphBuilder::VisitAssignment(Assignment*
expr) {
Node* key = environment()->Pop();
Node* object = environment()->Pop();
Node* store = BuildKeyedStore(object, key, value);
- PrepareFrameState(store, expr->id(),
ast_context()->GetStateCombine());
+ PrepareFrameStateAfterAndBefore(store, expr->id(),
+ ast_context()->GetStateCombine(),
+ frame_state_before_store);
break;
}
}
@@ -2176,6 +2200,11 @@ void
AstGraphBuilder::VisitCountOperation(CountOperation* expr) {
PrepareFrameState(old_value, expr->ToNumberId(),
OutputFrameStateCombine::Push());
+ Node* frame_state_before_store =
+ assign_type == KEYED_PROPERTY
+ ? environment()->Checkpoint(expr->ToNumberId())
+ : nullptr;
+
// Save result for postfix expressions at correct stack depth.
if (is_postfix) environment()->Poke(stack_depth, old_value);
@@ -2212,7 +2241,9 @@ void
AstGraphBuilder::VisitCountOperation(CountOperation* expr) {
Node* object = environment()->Pop();
Node* store = BuildKeyedStore(object, key, value);
environment()->Push(value);
- PrepareFrameState(store, expr->AssignmentId());
+ PrepareFrameStateAfterAndBefore(store, expr->AssignmentId(),
+ OutputFrameStateCombine::Ignore(),
+ frame_state_before_store);
environment()->Pop();
break;
}
Index: src/compiler/js-typed-lowering.cc
diff --git a/src/compiler/js-typed-lowering.cc
b/src/compiler/js-typed-lowering.cc
index
386518e9bd934f9b3ddfdbb46f3fb659e8409938..05363f288103f747fd0f4f171c608ed9a3e20d04
100644
--- a/src/compiler/js-typed-lowering.cc
+++ b/src/compiler/js-typed-lowering.cc
@@ -803,9 +803,11 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node*
node) {
(OperatorProperties::GetFrameStateInputCount(
javascript()->ToNumber()) == 1));
if (FLAG_turbo_deoptimization) {
+ Node* frame_state_for_to_number =
+ NodeProperties::GetFrameStateInput(node, 1);
value = effect =
graph()->NewNode(javascript()->ToNumber(), value, context,
- jsgraph()->EmptyFrameState(), effect,
control);
+ frame_state_for_to_number, effect,
control);
} else {
value = effect = graph()->NewNode(javascript()->ToNumber(),
value,
context, effect, control);
Index: src/compiler/operator-properties.cc
diff --git a/src/compiler/operator-properties.cc
b/src/compiler/operator-properties.cc
index
05891e1a743275dcd081c4cfe791e62775538007..3a91fd6b581b5e6a94e68f36df95efb4fd09ff0a
100644
--- a/src/compiler/operator-properties.cc
+++ b/src/compiler/operator-properties.cc
@@ -64,12 +64,17 @@ int OperatorProperties::GetFrameStateInputCount(const
Operator* op) {
// Properties
case IrOpcode::kJSLoadNamed:
- case IrOpcode::kJSLoadProperty:
case IrOpcode::kJSStoreNamed:
- case IrOpcode::kJSStoreProperty:
+ case IrOpcode::kJSLoadProperty:
case IrOpcode::kJSDeleteProperty:
return 1;
+ // StoreProperty provides a second frame state just before
+ // the operation. This is used to lazy-deoptimize a to-number
+ // conversion for typed arrays.
+ case IrOpcode::kJSStoreProperty:
+ return 2;
+
// Binary operators that can deopt in the middle the operation (e.g.,
// as a result of lazy deopt in ToNumber conversion) need a second
frame
// state so that we can resume before the operation.
Index: test/mjsunit/compiler/truncating-store-deopt.js
diff --git a/test/mjsunit/compiler/truncating-store-deopt.js
b/test/mjsunit/compiler/truncating-store-deopt.js
new file mode 100644
index
0000000000000000000000000000000000000000..a640caf58347d1341e4aa0f2d26d8c9aa373be0a
--- /dev/null
+++ b/test/mjsunit/compiler/truncating-store-deopt.js
@@ -0,0 +1,28 @@
+// Copyright 2015 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
+
+function g(a, b, c) {
+ return a + b + c;
+}
+
+var asm = (function Module(global, env, buffer) {
+ "use asm";
+
+ var i32 = new global.Int32Array(buffer);
+
+ // This is not valid asm.js, but we should still generate correct code.
+ function store(x) {
+ return g(1, i32[0] = x, 2);
+ }
+
+ return { store: store };
+})({
+ "Int32Array": Int32Array
+}, {}, new ArrayBuffer(64 * 1024));
+
+var o = { toString : function() { %DeoptimizeFunction(asm.store);
return "1"; } }
+
+asm.store(o);
Index: test/unittests/compiler/js-operator-unittest.cc
diff --git a/test/unittests/compiler/js-operator-unittest.cc
b/test/unittests/compiler/js-operator-unittest.cc
index
2cc3bfadd02b43dfc449868f04e61936a6697c16..f8ef7fbbb223516e1ac35a27991c5cff98002aba
100644
--- a/test/unittests/compiler/js-operator-unittest.cc
+++ b/test/unittests/compiler/js-operator-unittest.cc
@@ -171,7 +171,7 @@ TEST_P(JSStorePropertyOperatorTest,
NumberOfInputsAndOutputs) {
const Operator* op = javascript.StoreProperty(mode);
// TODO(jarin): Get rid of this hack.
- const int frame_state_input_count = FLAG_turbo_deoptimization ? 1 : 0;
+ const int frame_state_input_count = FLAG_turbo_deoptimization ? 2 : 0;
EXPECT_EQ(3, op->ValueInputCount());
EXPECT_EQ(1, OperatorProperties::GetContextInputCount(op));
EXPECT_EQ(frame_state_input_count,
Index: test/unittests/compiler/js-typed-lowering-unittest.cc
diff --git a/test/unittests/compiler/js-typed-lowering-unittest.cc
b/test/unittests/compiler/js-typed-lowering-unittest.cc
index
4fb4b2422be33b31d4f81e08d8d8c3d1c0711fb6..781cf4c2c65fdad1350258e64e470833e47100a9
100644
--- a/test/unittests/compiler/js-typed-lowering-unittest.cc
+++ b/test/unittests/compiler/js-typed-lowering-unittest.cc
@@ -8,6 +8,7 @@
#include "src/compiler/js-typed-lowering.h"
#include "src/compiler/machine-operator.h"
#include "src/compiler/node-properties.h"
+#include "src/compiler/operator-properties.h"
#include "test/unittests/compiler/compiler-test-utils.h"
#include "test/unittests/compiler/graph-unittest.h"
#include "test/unittests/compiler/node-test-utils.h"
@@ -691,8 +692,9 @@ TEST_F(JSTypedLoweringTest,
JSStorePropertyToExternalTypedArray) {
Node* control = graph()->start();
Node* node =
graph()->NewNode(javascript()->StoreProperty(language_mode),
base, key, value, context);
- if (FLAG_turbo_deoptimization) {
- node->AppendInput(zone(), UndefinedConstant());
+ for (int i = 0;
+ i < OperatorProperties::GetFrameStateInputCount(node->op());
i++) {
+ node->AppendInput(zone(), EmptyFrameState());
}
node->AppendInput(zone(), effect);
node->AppendInput(zone(), control);
@@ -736,8 +738,9 @@ TEST_F(JSTypedLoweringTest,
JSStorePropertyToExternalTypedArrayWithConversion) {
Node* control = graph()->start();
Node* node =
graph()->NewNode(javascript()->StoreProperty(language_mode),
base, key, value, context);
- if (FLAG_turbo_deoptimization) {
- node->AppendInput(zone(), UndefinedConstant());
+ for (int i = 0;
+ i < OperatorProperties::GetFrameStateInputCount(node->op());
i++) {
+ node->AppendInput(zone(), EmptyFrameState());
}
node->AppendInput(zone(), effect);
node->AppendInput(zone(), control);
@@ -794,8 +797,9 @@ TEST_F(JSTypedLoweringTest,
JSStorePropertyToExternalTypedArrayWithSafeKey) {
Node* control = graph()->start();
Node* node =
graph()->NewNode(javascript()->StoreProperty(language_mode),
base, key, value, context);
- if (FLAG_turbo_deoptimization) {
- node->AppendInput(zone(), UndefinedConstant());
+ for (int i = 0;
+ i < OperatorProperties::GetFrameStateInputCount(node->op());
i++) {
+ node->AppendInput(zone(), EmptyFrameState());
}
node->AppendInput(zone(), effect);
node->AppendInput(zone(), control);
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
