Commit ac04d777 authored by jarin's avatar jarin Committed by Commit bot

[turbofan] Allow deoptimization for JSToNumber operator.

R=bmeurer@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#26053}
parent 5d22f59d
......@@ -4676,6 +4676,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
ToNumberStub convert_stub(isolate());
__ CallStub(&convert_stub);
PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
// Save result for postfix expressions.
if (expr->is_postfix()) {
......
......@@ -4360,6 +4360,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
ToNumberStub convert_stub(isolate());
__ CallStub(&convert_stub);
PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
// Save result for postfix expressions.
if (expr->is_postfix()) {
......
......@@ -2170,13 +2170,14 @@ class CountOperation FINAL : public Expression {
}
void set_type(Type* type) { type_ = type; }
static int num_ids() { return parent_num_ids() + 3; }
static int num_ids() { return parent_num_ids() + 4; }
BailoutId AssignmentId() const { return BailoutId(local_id(0)); }
BailoutId ToNumberId() const { return BailoutId(local_id(1)); }
TypeFeedbackId CountBinOpFeedbackId() const {
return TypeFeedbackId(local_id(1));
return TypeFeedbackId(local_id(2));
}
TypeFeedbackId CountStoreFeedbackId() const {
return TypeFeedbackId(local_id(2));
return TypeFeedbackId(local_id(3));
}
protected:
......
......@@ -1488,6 +1488,8 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) {
// Convert old value into a number.
old_value = NewNode(javascript()->ToNumber(), old_value);
PrepareFrameState(old_value, expr->ToNumberId(),
OutputFrameStateCombine::Push());
// Save result for postfix expressions at correct stack depth.
if (is_postfix) environment()->Poke(stack_depth, old_value);
......
......@@ -227,7 +227,12 @@ Reduction ChangeLowering::ChangeTaggedToFloat64(Node* value, Node* control) {
d1.Chain(control);
Node* number =
graph()->NewNode(value->op(), object, context, effect, d1.if_true);
OperatorProperties::HasFrameStateInput(value->op())
? graph()->NewNode(value->op(), object, context,
NodeProperties::GetFrameStateInput(value),
effect, d1.if_true)
: graph()->NewNode(value->op(), object, context, effect,
d1.if_true);
Diamond d2(graph(), common(), TestNotSmi(number));
d2.Nest(d1, true);
Node* phi2 = d2.Phi(kMachFloat64, LoadHeapNumberValue(number, d2.if_true),
......
......@@ -191,6 +191,19 @@ Node* JSGraph::Float64Constant(double value) {
}
Node* JSGraph::EmptyFrameState() {
if (!empty_frame_state_.is_set()) {
Node* values = graph()->NewNode(common()->StateValues(0));
Node* state_node = graph()->NewNode(
common()->FrameState(JS_FRAME, BailoutId(0),
OutputFrameStateCombine::Ignore()),
values, values, values, NoContextConstant(), UndefinedConstant());
empty_frame_state_.set(state_node);
}
return empty_frame_state_.get();
}
Node* JSGraph::ExternalConstant(ExternalReference reference) {
Node** loc = cache_.FindExternalConstant(reference);
if (*loc == NULL) {
......
......@@ -109,6 +109,10 @@ class JSGraph : public ZoneObject {
// stubs and runtime functions that do not require a context.
Node* NoContextConstant() { return ZeroConstant(); }
// Creates an empty frame states for cases where we know that a function
// cannot deopt.
Node* EmptyFrameState();
JSOperatorBuilder* javascript() { return javascript_; }
CommonOperatorBuilder* common() { return common_; }
MachineOperatorBuilder* machine() { return machine_; }
......@@ -135,6 +139,7 @@ class JSGraph : public ZoneObject {
SetOncePointer<Node> zero_constant_;
SetOncePointer<Node> one_constant_;
SetOncePointer<Node> nan_constant_;
SetOncePointer<Node> empty_frame_state_;
CommonNodeCache cache_;
......
......@@ -198,8 +198,16 @@ class JSBinopReduction FINAL {
if (NodeProperties::GetBounds(node).upper->Is(Type::PlainPrimitive())) {
return lowering_->ConvertToNumber(node);
}
Node* n = graph()->NewNode(javascript()->ToNumber(), node, context(),
effect(), control());
// TODO(jarin) This ToNumber conversion can deoptimize, but we do not really
// have a frame state to deoptimize to. Either we provide such a frame state
// or we exclude the values that could lead to deoptimization (e.g., by
// triggering eager deopt if the value is not plain).
Node* const n = FLAG_turbo_deoptimization
? graph()->NewNode(
javascript()->ToNumber(), node, context(),
jsgraph()->EmptyFrameState(), effect(), control())
: graph()->NewNode(javascript()->ToNumber(), node,
context(), effect(), control());
update_effect(n);
return n;
}
......@@ -624,15 +632,20 @@ Reduction JSTypedLowering::ReduceJSToNumber(Node* node) {
}
// Remember this conversion.
InsertConversion(node);
if (node->InputAt(1) != jsgraph()->NoContextConstant() ||
node->InputAt(2) != graph()->start() ||
node->InputAt(3) != graph()->start()) {
if (NodeProperties::GetContextInput(node) !=
jsgraph()->NoContextConstant() ||
NodeProperties::GetEffectInput(node) != graph()->start() ||
NodeProperties::GetControlInput(node) != graph()->start()) {
// JSToNumber(x:plain-primitive,context,effect,control)
// => JSToNumber(x,no-context,start,start)
RelaxEffects(node);
node->ReplaceInput(1, jsgraph()->NoContextConstant());
node->ReplaceInput(2, graph()->start());
node->ReplaceInput(3, graph()->start());
NodeProperties::ReplaceContextInput(node, jsgraph()->NoContextConstant());
NodeProperties::ReplaceControlInput(node, graph()->start());
NodeProperties::ReplaceEffectInput(node, graph()->start());
if (OperatorProperties::HasFrameStateInput(node->op())) {
NodeProperties::ReplaceFrameStateInput(node,
jsgraph()->EmptyFrameState());
}
return Changed(node);
}
}
......@@ -752,8 +765,15 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node* node) {
if (number_reduction.Changed()) {
value = number_reduction.replacement();
} else {
value = effect = graph()->NewNode(javascript()->ToNumber(), value,
context, effect, control);
if (OperatorProperties::HasFrameStateInput(
javascript()->ToNumber())) {
value = effect =
graph()->NewNode(javascript()->ToNumber(), value, context,
jsgraph()->EmptyFrameState(), effect, control);
} else {
value = effect = graph()->NewNode(javascript()->ToNumber(), value,
context, effect, control);
}
}
}
// For integer-typed arrays, convert to the integer type.
......@@ -785,8 +805,8 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node* node) {
node->ReplaceInput(2, length);
node->ReplaceInput(3, value);
node->ReplaceInput(4, effect);
DCHECK_EQ(control, node->InputAt(5));
DCHECK_EQ(6, node->InputCount());
node->ReplaceInput(5, control);
node->TrimInputCount(6);
return Changed(node);
}
}
......@@ -932,9 +952,16 @@ Node* JSTypedLowering::ConvertToNumber(Node* input) {
// Avoid inserting too many eager ToNumber() operations.
Reduction const reduction = ReduceJSToNumberInput(input);
if (reduction.Changed()) return reduction.replacement();
Node* const conversion = graph()->NewNode(javascript()->ToNumber(), input,
jsgraph()->NoContextConstant(),
graph()->start(), graph()->start());
// TODO(jarin) Use PlainPrimitiveToNumber once we have it.
Node* const conversion =
FLAG_turbo_deoptimization
? graph()->NewNode(javascript()->ToNumber(), input,
jsgraph()->NoContextConstant(),
jsgraph()->EmptyFrameState(), graph()->start(),
graph()->start())
: graph()->NewNode(javascript()->ToNumber(), input,
jsgraph()->NoContextConstant(), graph()->start(),
graph()->start());
InsertConversion(conversion);
return conversion;
}
......
......@@ -144,6 +144,10 @@ inline bool NodeProperties::IsControl(Node* node) {
// -----------------------------------------------------------------------------
// Miscellaneous mutators.
inline void NodeProperties::ReplaceContextInput(Node* node, Node* context) {
node->ReplaceInput(FirstContextIndex(node), context);
}
inline void NodeProperties::ReplaceControlInput(Node* node, Node* control) {
node->ReplaceInput(FirstControlIndex(node), control);
}
......
......@@ -32,6 +32,7 @@ class NodeProperties {
static inline bool IsControl(Node* node);
static inline void ReplaceContextInput(Node* node, Node* context);
static inline void ReplaceControlInput(Node* node, Node* control);
static inline void ReplaceEffectInput(Node* node, Node* effect,
int index = 0);
......
......@@ -70,6 +70,7 @@ bool OperatorProperties::HasFrameStateInput(const Operator* op) {
// Conversions
case IrOpcode::kJSToObject:
case IrOpcode::kJSToNumber:
// Other
case IrOpcode::kJSDeleteProperty:
......
......@@ -4621,6 +4621,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
ToNumberStub convert_stub(isolate());
__ CallStub(&convert_stub);
PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
// Save result for postfix expressions.
if (expr->is_postfix()) {
......
......@@ -4681,6 +4681,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
ToNumberStub convert_stub(isolate());
__ CallStub(&convert_stub);
PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
// Save result for postfix expressions.
if (expr->is_postfix()) {
......
......@@ -4681,6 +4681,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
ToNumberStub convert_stub(isolate());
__ CallStub(&convert_stub);
PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
// Save result for postfix expressions.
if (expr->is_postfix()) {
......
......@@ -4730,6 +4730,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
ToNumberStub convert_stub(isolate());
__ CallStub(&convert_stub);
PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
// Save result for postfix expressions.
if (expr->is_postfix()) {
......
......@@ -4636,6 +4636,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
ToNumberStub convert_stub(isolate());
__ CallStub(&convert_stub);
PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
// Save result for postfix expressions.
if (expr->is_postfix()) {
......
......@@ -4607,6 +4607,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
ToNumberStub convert_stub(isolate());
__ CallStub(&convert_stub);
PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
// Save result for postfix expressions.
if (expr->is_postfix()) {
......
......@@ -123,8 +123,13 @@ class JSTypedLoweringTester : public HandleAndZoneScope {
Node* UseForEffect(Node* node) {
// TODO(titzer): use EffectPhi after fixing EffectCount
return graph.NewNode(javascript.ToNumber(), node, context(), node,
control());
if (OperatorProperties::HasFrameStateInput(javascript.ToNumber())) {
return graph.NewNode(javascript.ToNumber(), node, context(),
EmptyFrameState(context()), node, control());
} else {
return graph.NewNode(javascript.ToNumber(), node, context(), node,
control());
}
}
void CheckEffectInput(Node* effect, Node* use) {
......@@ -737,12 +742,25 @@ TEST(RemoveToNumberEffects) {
switch (i) {
case 0:
// TODO(jarin) Replace with a query of FLAG_turbo_deoptimization.
if (OperatorProperties::HasFrameStateInput(R.javascript.ToNumber())) {
effect_use = R.graph.NewNode(R.javascript.ToNumber(), p0, R.context(),
frame_state, ton, R.start());
} else {
effect_use = R.graph.NewNode(R.javascript.ToNumber(), p0, R.context(),
ton, R.start());
}
break;
case 1:
effect_use = R.graph.NewNode(R.javascript.ToNumber(), ton, R.context(),
ton, R.start());
// TODO(jarin) Replace with a query of FLAG_turbo_deoptimization.
if (OperatorProperties::HasFrameStateInput(R.javascript.ToNumber())) {
effect_use =
R.graph.NewNode(R.javascript.ToNumber(), ton, R.context(),
frame_state, ton, R.start());
} else {
effect_use = R.graph.NewNode(R.javascript.ToNumber(), ton,
R.context(), ton, R.start());
}
break;
case 2:
effect_use = R.graph.NewNode(R.common.EffectPhi(1), ton, R.start());
......
// Copyright 2014 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: --turbo-deoptimization
v = [];
v.length = (1 << 30);
function f() {
v++;
}
assertThrows(f);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment