Commit b947fff8 authored by bmeurer's avatar bmeurer Committed by Commit bot

[turbofan] Don't generate unnecessary minus zero checks.

When we do a checked conversion from Tagged or Float64 to Int32, we used
to always do a minus zero check, even if we already know that the input
cannot be minus zero. Now we actually do the check only if we have
evidence that the input might be minus zero.

R=epertoso@chromium.org
BUG=v8:4583

Review-Url: https://codereview.chromium.org/2202993005
Cr-Commit-Position: refs/heads/master@{#38308}
parent 0872d08b
......@@ -1532,7 +1532,8 @@ EffectControlLinearizer::LowerCheckedUint32ToInt32(Node* node,
}
EffectControlLinearizer::ValueEffectControl
EffectControlLinearizer::BuildCheckedFloat64ToInt32(Node* value,
EffectControlLinearizer::BuildCheckedFloat64ToInt32(CheckForMinusZeroMode mode,
Node* value,
Node* frame_state,
Node* effect,
Node* control) {
......@@ -1544,32 +1545,33 @@ EffectControlLinearizer::BuildCheckedFloat64ToInt32(Node* value,
common()->DeoptimizeUnless(DeoptimizeReason::kLostPrecisionOrNaN),
check_same, frame_state, effect, control);
// Check if {value} is -0.
Node* check_zero = graph()->NewNode(machine()->Word32Equal(), value32,
jsgraph()->Int32Constant(0));
Node* branch_zero = graph()->NewNode(common()->Branch(BranchHint::kFalse),
check_zero, control);
Node* if_zero = graph()->NewNode(common()->IfTrue(), branch_zero);
Node* if_notzero = graph()->NewNode(common()->IfFalse(), branch_zero);
if (mode == CheckForMinusZeroMode::kCheckForMinusZero) {
// Check if {value} is -0.
Node* check_zero = graph()->NewNode(machine()->Word32Equal(), value32,
jsgraph()->Int32Constant(0));
Node* branch_zero = graph()->NewNode(common()->Branch(BranchHint::kFalse),
check_zero, control);
// In case of 0, we need to check the high bits for the IEEE -0 pattern.
Node* check_negative = graph()->NewNode(
machine()->Int32LessThan(),
graph()->NewNode(machine()->Float64ExtractHighWord32(), value),
jsgraph()->Int32Constant(0));
Node* if_zero = graph()->NewNode(common()->IfTrue(), branch_zero);
Node* if_notzero = graph()->NewNode(common()->IfFalse(), branch_zero);
Node* deopt_minus_zero =
graph()->NewNode(common()->DeoptimizeIf(DeoptimizeReason::kMinusZero),
check_negative, frame_state, effect, if_zero);
// In case of 0, we need to check the high bits for the IEEE -0 pattern.
Node* check_negative = graph()->NewNode(
machine()->Int32LessThan(),
graph()->NewNode(machine()->Float64ExtractHighWord32(), value),
jsgraph()->Int32Constant(0));
Node* merge =
graph()->NewNode(common()->Merge(2), deopt_minus_zero, if_notzero);
Node* deopt_minus_zero =
graph()->NewNode(common()->DeoptimizeIf(DeoptimizeReason::kMinusZero),
check_negative, frame_state, effect, if_zero);
effect =
graph()->NewNode(common()->EffectPhi(2), deopt_minus_zero, effect, merge);
control =
graph()->NewNode(common()->Merge(2), deopt_minus_zero, if_notzero);
effect = graph()->NewNode(common()->EffectPhi(2), deopt_minus_zero, effect,
control);
}
return ValueEffectControl(value32, effect, merge);
return ValueEffectControl(value32, effect, control);
}
EffectControlLinearizer::ValueEffectControl
......@@ -1577,9 +1579,10 @@ EffectControlLinearizer::LowerCheckedFloat64ToInt32(Node* node,
Node* frame_state,
Node* effect,
Node* control) {
CheckForMinusZeroMode mode = CheckMinusZeroModeOf(node->op());
Node* value = node->InputAt(0);
return BuildCheckedFloat64ToInt32(value, frame_state, effect, control);
return BuildCheckedFloat64ToInt32(mode, value, frame_state, effect, control);
}
EffectControlLinearizer::ValueEffectControl
......@@ -1603,6 +1606,7 @@ EffectControlLinearizer::LowerCheckedTaggedToInt32(Node* node,
Node* frame_state,
Node* effect,
Node* control) {
CheckForMinusZeroMode mode = CheckMinusZeroModeOf(node->op());
Node* value = node->InputAt(0);
Node* check = ObjectIsSmi(value);
......@@ -1632,7 +1636,7 @@ EffectControlLinearizer::LowerCheckedTaggedToInt32(Node* node,
simplified()->LoadField(AccessBuilder::ForHeapNumberValue()), value,
efalse, if_false);
ValueEffectControl state =
BuildCheckedFloat64ToInt32(vfalse, frame_state, efalse, if_false);
BuildCheckedFloat64ToInt32(mode, vfalse, frame_state, efalse, if_false);
if_false = state.control;
efalse = state.effect;
vfalse = state.value;
......
......@@ -145,7 +145,8 @@ class EffectControlLinearizer {
ValueEffectControl AllocateHeapNumberWithValue(Node* node, Node* effect,
Node* control);
ValueEffectControl BuildCheckedFloat64ToInt32(Node* value, Node* frame_state,
ValueEffectControl BuildCheckedFloat64ToInt32(CheckForMinusZeroMode mode,
Node* value, Node* frame_state,
Node* effect, Node* control);
ValueEffectControl BuildCheckedHeapNumberOrOddballToFloat64(Node* value,
Node* frame_state,
......
......@@ -439,7 +439,10 @@ Node* RepresentationChanger::GetWord32RepresentationFor(
op = machine()->TruncateFloat64ToWord32();
} else if (use_info.type_check() == TypeCheckKind::kSignedSmall ||
use_info.type_check() == TypeCheckKind::kSigned32) {
op = simplified()->CheckedFloat64ToInt32();
op = simplified()->CheckedFloat64ToInt32(
output_type->Maybe(Type::MinusZero())
? CheckForMinusZeroMode::kCheckForMinusZero
: CheckForMinusZeroMode::kDontCheckForMinusZero);
}
} else if (output_rep == MachineRepresentation::kFloat32) {
node = InsertChangeFloat32ToFloat64(node); // float32 -> float64 -> int32
......@@ -451,7 +454,10 @@ Node* RepresentationChanger::GetWord32RepresentationFor(
op = machine()->TruncateFloat64ToWord32();
} else if (use_info.type_check() == TypeCheckKind::kSignedSmall ||
use_info.type_check() == TypeCheckKind::kSigned32) {
op = simplified()->CheckedFloat64ToInt32();
op = simplified()->CheckedFloat64ToInt32(
output_type->Maybe(Type::MinusZero())
? CheckForMinusZeroMode::kCheckForMinusZero
: CheckForMinusZeroMode::kDontCheckForMinusZero);
}
} else if (output_rep == MachineRepresentation::kTagged) {
if (output_type->Is(Type::TaggedSigned())) {
......@@ -469,7 +475,10 @@ Node* RepresentationChanger::GetWord32RepresentationFor(
} else if (use_info.type_check() == TypeCheckKind::kSignedSmall) {
op = simplified()->CheckedTaggedSignedToInt32();
} else if (use_info.type_check() == TypeCheckKind::kSigned32) {
op = simplified()->CheckedTaggedToInt32();
op = simplified()->CheckedTaggedToInt32(
output_type->Maybe(Type::MinusZero())
? CheckForMinusZeroMode::kCheckForMinusZero
: CheckForMinusZeroMode::kDontCheckForMinusZero);
}
} else if (output_rep == MachineRepresentation::kWord32) {
// Only the checked case should get here, the non-checked case is
......
......@@ -208,7 +208,9 @@ CheckFloat64HoleMode CheckFloat64HoleModeOf(const Operator* op) {
}
CheckForMinusZeroMode CheckMinusZeroModeOf(const Operator* op) {
DCHECK_EQ(IrOpcode::kCheckedInt32Mul, op->opcode());
DCHECK(op->opcode() == IrOpcode::kCheckedInt32Mul ||
op->opcode() == IrOpcode::kCheckedFloat64ToInt32 ||
op->opcode() == IrOpcode::kCheckedTaggedToInt32);
return OpParameter<CheckForMinusZeroMode>(op);
}
......@@ -395,9 +397,7 @@ CompareOperationHints::Hint CompareOperationHintOf(const Operator* op) {
V(CheckedUint32Div, 2, 1) \
V(CheckedUint32Mod, 2, 1) \
V(CheckedUint32ToInt32, 1, 1) \
V(CheckedFloat64ToInt32, 1, 1) \
V(CheckedTaggedSignedToInt32, 1, 1) \
V(CheckedTaggedToInt32, 1, 1) \
V(CheckedTaggedToFloat64, 1, 1) \
V(CheckedTruncateTaggedToWord32, 1, 1)
......@@ -437,6 +437,34 @@ struct SimplifiedOperatorGlobalCache final {
CheckedInt32MulOperator<CheckForMinusZeroMode::kDontCheckForMinusZero>
kCheckedInt32MulDontCheckForMinusZeroOperator;
template <CheckForMinusZeroMode kMode>
struct CheckedFloat64ToInt32Operator final
: public Operator1<CheckForMinusZeroMode> {
CheckedFloat64ToInt32Operator()
: Operator1<CheckForMinusZeroMode>(
IrOpcode::kCheckedFloat64ToInt32,
Operator::kFoldable | Operator::kNoThrow, "CheckedFloat64ToInt32",
1, 1, 1, 1, 1, 0, kMode) {}
};
CheckedFloat64ToInt32Operator<CheckForMinusZeroMode::kCheckForMinusZero>
kCheckedFloat64ToInt32CheckForMinusZeroOperator;
CheckedFloat64ToInt32Operator<CheckForMinusZeroMode::kDontCheckForMinusZero>
kCheckedFloat64ToInt32DontCheckForMinusZeroOperator;
template <CheckForMinusZeroMode kMode>
struct CheckedTaggedToInt32Operator final
: public Operator1<CheckForMinusZeroMode> {
CheckedTaggedToInt32Operator()
: Operator1<CheckForMinusZeroMode>(
IrOpcode::kCheckedTaggedToInt32,
Operator::kFoldable | Operator::kNoThrow, "CheckedTaggedToInt32",
1, 1, 1, 1, 1, 0, kMode) {}
};
CheckedTaggedToInt32Operator<CheckForMinusZeroMode::kCheckForMinusZero>
kCheckedTaggedToInt32CheckForMinusZeroOperator;
CheckedTaggedToInt32Operator<CheckForMinusZeroMode::kDontCheckForMinusZero>
kCheckedTaggedToInt32DontCheckForMinusZeroOperator;
template <CheckFloat64HoleMode kMode>
struct CheckFloat64HoleNaNOperator final
: public Operator1<CheckFloat64HoleMode> {
......@@ -524,6 +552,30 @@ const Operator* SimplifiedOperatorBuilder::CheckedInt32Mul(
return nullptr;
}
const Operator* SimplifiedOperatorBuilder::CheckedFloat64ToInt32(
CheckForMinusZeroMode mode) {
switch (mode) {
case CheckForMinusZeroMode::kCheckForMinusZero:
return &cache_.kCheckedFloat64ToInt32CheckForMinusZeroOperator;
case CheckForMinusZeroMode::kDontCheckForMinusZero:
return &cache_.kCheckedFloat64ToInt32DontCheckForMinusZeroOperator;
}
UNREACHABLE();
return nullptr;
}
const Operator* SimplifiedOperatorBuilder::CheckedTaggedToInt32(
CheckForMinusZeroMode mode) {
switch (mode) {
case CheckForMinusZeroMode::kCheckForMinusZero:
return &cache_.kCheckedTaggedToInt32CheckForMinusZeroOperator;
case CheckForMinusZeroMode::kDontCheckForMinusZero:
return &cache_.kCheckedTaggedToInt32DontCheckForMinusZeroOperator;
}
UNREACHABLE();
return nullptr;
}
const Operator* SimplifiedOperatorBuilder::CheckMaps(int map_input_count) {
// TODO(bmeurer): Cache the most important versions of this operator.
DCHECK_LT(0, map_input_count);
......
......@@ -295,9 +295,9 @@ class SimplifiedOperatorBuilder final : public ZoneObject {
const Operator* CheckedUint32Mod();
const Operator* CheckedInt32Mul(CheckForMinusZeroMode);
const Operator* CheckedUint32ToInt32();
const Operator* CheckedFloat64ToInt32();
const Operator* CheckedFloat64ToInt32(CheckForMinusZeroMode);
const Operator* CheckedTaggedSignedToInt32();
const Operator* CheckedTaggedToInt32();
const Operator* CheckedTaggedToInt32(CheckForMinusZeroMode);
const Operator* CheckedTaggedToFloat64();
const Operator* CheckedTruncateTaggedToWord32();
......
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