Commit c54d93d6 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[turbofan] Fix inconsistent treatment of SpeculativeToNumber.

This is a partial revert of e583fc83.
The reasoning here is that the treatment of SpeculativeToNumber[hint]
was not consistent (which led to the original bug that caused the
performance regression): The semantics of the operator is that it turns
its input into a number, and might bailout if the input is too complex
to accomplish that within optimized code. It can use the hint to handle
even fewer cases without the risk of a deoptimization loop. However it
cannot rely on the hint influencing the output, especially not before
SimplifiedLowering ran. The code for the OOB element access however
relied on the hint being enforced, which caused the original bug.

This CL repairs that and instead uses CheckSmi for the OOB element
access guard.

Also-By: tebbi@chromium.org
Bug: chromium:819298, chromium:820729
Change-Id: I9b2170ccf9b5561d698c0108e93e538cac1e708c
Reviewed-on: https://chromium-review.googlesource.com/961066Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51924}
parent 23d7fb69
...@@ -2246,13 +2246,11 @@ JSNativeContextSpecialization::BuildElementAccess( ...@@ -2246,13 +2246,11 @@ JSNativeContextSpecialization::BuildElementAccess(
if (load_mode == LOAD_IGNORE_OUT_OF_BOUNDS || if (load_mode == LOAD_IGNORE_OUT_OF_BOUNDS ||
store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) { store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) {
// Only check that the {index} is in Signed32 range. We do the actual // Only check that the {index} is in SignedSmall range. We do the actual
// bounds check below and just skip the property access if it's out of // bounds check below and just skip the property access if it's out of
// bounds for the {receiver}. // bounds for the {receiver}.
index = effect = graph()->NewNode( index = effect = graph()->NewNode(
simplified()->SpeculativeToNumber(NumberOperationHint::kSigned32, simplified()->CheckSmi(VectorSlotPair()), index, effect, control);
VectorSlotPair()),
index, effect, control);
// Cast the {index} to Unsigned32 range, so that the bounds checks // Cast the {index} to Unsigned32 range, so that the bounds checks
// below are performed on unsigned values, which means that all the // below are performed on unsigned values, which means that all the
......
...@@ -504,30 +504,10 @@ Reduction TypedOptimization::ReduceSpeculativeToNumber(Node* node) { ...@@ -504,30 +504,10 @@ Reduction TypedOptimization::ReduceSpeculativeToNumber(Node* node) {
DCHECK_EQ(IrOpcode::kSpeculativeToNumber, node->opcode()); DCHECK_EQ(IrOpcode::kSpeculativeToNumber, node->opcode());
Node* const input = NodeProperties::GetValueInput(node, 0); Node* const input = NodeProperties::GetValueInput(node, 0);
Type* const input_type = NodeProperties::GetType(input); Type* const input_type = NodeProperties::GetType(input);
switch (NumberOperationParametersOf(node->op()).hint()) { if (input_type->Is(Type::Number())) {
case NumberOperationHint::kSigned32: // SpeculativeToNumber(x:number) => x
if (input_type->Is(Type::Signed32())) { ReplaceWithValue(node, input);
// SpeculativeToNumber(x:signed32) => x return Replace(input);
ReplaceWithValue(node, input);
return Replace(input);
}
break;
case NumberOperationHint::kSignedSmall:
case NumberOperationHint::kSignedSmallInputs:
if (input_type->Is(Type::SignedSmall())) {
// SpeculativeToNumber(x:signed-small) => x
ReplaceWithValue(node, input);
return Replace(input);
}
break;
case NumberOperationHint::kNumber:
case NumberOperationHint::kNumberOrOddball:
if (input_type->Is(Type::Number())) {
// SpeculativeToNumber(x:number) => x
ReplaceWithValue(node, input);
return Replace(input);
}
break;
} }
return NoChange(); return NoChange();
} }
......
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