Commit bba8024d authored by georgia.kouveli's avatar georgia.kouveli Committed by Commit bot

[turbofan] Remove minus zero check for rhs of CheckedInt32Sub.

The only way to get a minus zero result from subtraction is
(-0) - (+0) = -0, hence checking for minus zero on the RHS is
redundant. This is causing some unnecessary deoptimisations
in Box2D from Octane on 32-bit platforms.

BUG=

Review-Url: https://codereview.chromium.org/2410883003
Cr-Commit-Position: refs/heads/master@{#40207}
parent c15c5827
......@@ -587,7 +587,7 @@ Node* RepresentationChanger::GetWord32RepresentationFor(
use_info.type_check() == TypeCheckKind::kSigned32) {
op = simplified()->CheckedFloat64ToInt32(
output_type->Maybe(Type::MinusZero())
? CheckForMinusZeroMode::kCheckForMinusZero
? use_info.minus_zero_check()
: CheckForMinusZeroMode::kDontCheckForMinusZero);
}
} else if (output_rep == MachineRepresentation::kFloat32) {
......
......@@ -108,7 +108,8 @@ inline std::ostream& operator<<(std::ostream& os, TypeCheckKind type_check) {
//
// 1. During propagation, the use info is used to inform the input node
// about what part of the input is used (we call this truncation) and what
// is the preferred representation.
// is the preferred representation. For conversions that will require
// checks, we also keep track of whether a minus zero check is needed.
//
// 2. During lowering, the use info is used to properly convert the input
// to the preferred representation. The preferred representation might be
......@@ -117,10 +118,13 @@ inline std::ostream& operator<<(std::ostream& os, TypeCheckKind type_check) {
class UseInfo {
public:
UseInfo(MachineRepresentation representation, Truncation truncation,
TypeCheckKind type_check = TypeCheckKind::kNone)
TypeCheckKind type_check = TypeCheckKind::kNone,
CheckForMinusZeroMode minus_zero_check =
CheckForMinusZeroMode::kCheckForMinusZero)
: representation_(representation),
truncation_(truncation),
type_check_(type_check) {}
type_check_(type_check),
minus_zero_check_(minus_zero_check) {}
static UseInfo TruncatingWord32() {
return UseInfo(MachineRepresentation::kWord32, Truncation::Word32());
}
......@@ -154,13 +158,17 @@ class UseInfo {
return UseInfo(MachineRepresentation::kTaggedSigned, Truncation::Any(),
TypeCheckKind::kSignedSmall);
}
static UseInfo CheckedSignedSmallAsWord32() {
static UseInfo CheckedSignedSmallAsWord32(
CheckForMinusZeroMode minus_zero_mode =
CheckForMinusZeroMode::kCheckForMinusZero) {
return UseInfo(MachineRepresentation::kWord32, Truncation::Any(),
TypeCheckKind::kSignedSmall);
TypeCheckKind::kSignedSmall, minus_zero_mode);
}
static UseInfo CheckedSigned32AsWord32() {
static UseInfo CheckedSigned32AsWord32(
CheckForMinusZeroMode minus_zero_mode =
CheckForMinusZeroMode::kCheckForMinusZero) {
return UseInfo(MachineRepresentation::kWord32, Truncation::Any(),
TypeCheckKind::kSigned32);
TypeCheckKind::kSigned32, minus_zero_mode);
}
static UseInfo CheckedNumberAsFloat64() {
return UseInfo(MachineRepresentation::kFloat64, Truncation::Float64(),
......@@ -195,11 +203,14 @@ class UseInfo {
MachineRepresentation representation() const { return representation_; }
Truncation truncation() const { return truncation_; }
TypeCheckKind type_check() const { return type_check_; }
CheckForMinusZeroMode minus_zero_check() const { return minus_zero_check_; }
private:
MachineRepresentation representation_;
Truncation truncation_;
TypeCheckKind type_check_;
// TODO(jarin) Integrate with truncations.
CheckForMinusZeroMode minus_zero_check_;
};
// Contains logic related to changing the representation of values for constants
......
......@@ -87,12 +87,14 @@ MachineRepresentation MachineRepresentationFromArrayType(
return MachineRepresentation::kNone;
}
UseInfo CheckedUseInfoAsWord32FromHint(NumberOperationHint hint) {
UseInfo CheckedUseInfoAsWord32FromHint(
NumberOperationHint hint, CheckForMinusZeroMode minus_zero_mode =
CheckForMinusZeroMode::kCheckForMinusZero) {
switch (hint) {
case NumberOperationHint::kSignedSmall:
return UseInfo::CheckedSignedSmallAsWord32();
return UseInfo::CheckedSignedSmallAsWord32(minus_zero_mode);
case NumberOperationHint::kSigned32:
return UseInfo::CheckedSigned32AsWord32();
return UseInfo::CheckedSigned32AsWord32(minus_zero_mode);
case NumberOperationHint::kNumber:
return UseInfo::CheckedNumberAsWord32();
case NumberOperationHint::kNumberOrOddball:
......@@ -1146,8 +1148,15 @@ class RepresentationSelector {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
MachineRepresentation::kWord32, Type::Signed32());
UseInfo left_use = CheckedUseInfoAsWord32FromHint(hint);
// For subtraction, the right hand side can be minus zero without
// resulting in minus zero, so we skip the check for it.
UseInfo right_use = CheckedUseInfoAsWord32FromHint(
hint, node->opcode() == IrOpcode::kSpeculativeNumberSubtract
? CheckForMinusZeroMode::kDontCheckForMinusZero
: CheckForMinusZeroMode::kCheckForMinusZero);
VisitBinop(node, left_use, right_use, MachineRepresentation::kWord32,
Type::Signed32());
if (lower()) ChangeToInt32OverflowOp(node);
return;
}
......
......@@ -443,19 +443,31 @@ TEST(ToUint32_constant) {
}
static void CheckChange(IrOpcode::Value expected, MachineRepresentation from,
Type* from_type, MachineRepresentation to) {
Type* from_type, UseInfo use_info) {
RepresentationChangerTester r;
Node* n = r.Parameter();
Node* use = r.Return(n);
Node* c = r.changer()->GetRepresentationFor(n, from, from_type, use,
UseInfo(to, Truncation::None()));
Node* c =
r.changer()->GetRepresentationFor(n, from, from_type, use, use_info);
CHECK_NE(c, n);
CHECK_EQ(expected, c->opcode());
CHECK_EQ(n, c->InputAt(0));
if (expected == IrOpcode::kCheckedFloat64ToInt32) {
CheckForMinusZeroMode mode =
from_type->Maybe(Type::MinusZero())
? use_info.minus_zero_check()
: CheckForMinusZeroMode::kDontCheckForMinusZero;
CHECK_EQ(mode, CheckMinusZeroModeOf(c->op()));
}
}
static void CheckChange(IrOpcode::Value expected, MachineRepresentation from,
Type* from_type, MachineRepresentation to) {
CheckChange(expected, from, from_type, UseInfo(to, Truncation::None()));
}
static void CheckTwoChanges(IrOpcode::Value expected2,
IrOpcode::Value expected1,
......@@ -604,6 +616,32 @@ TEST(SignednessInWord32) {
MachineRepresentation::kWord32);
}
static void TestMinusZeroCheck(IrOpcode::Value expected, Type* from_type) {
RepresentationChangerTester r;
CheckChange(expected, MachineRepresentation::kFloat64, from_type,
UseInfo::CheckedSignedSmallAsWord32(
CheckForMinusZeroMode::kCheckForMinusZero));
CheckChange(expected, MachineRepresentation::kFloat64, from_type,
UseInfo::CheckedSignedSmallAsWord32(
CheckForMinusZeroMode::kDontCheckForMinusZero));
CheckChange(expected, MachineRepresentation::kFloat64, from_type,
UseInfo::CheckedSigned32AsWord32(
CheckForMinusZeroMode::kCheckForMinusZero));
CheckChange(expected, MachineRepresentation::kFloat64, from_type,
UseInfo::CheckedSigned32AsWord32(
CheckForMinusZeroMode::kDontCheckForMinusZero));
}
TEST(MinusZeroCheck) {
TestMinusZeroCheck(IrOpcode::kCheckedFloat64ToInt32, Type::NumberOrOddball());
// PlainNumber cannot be minus zero so the minus zero check should be
// eliminated.
TestMinusZeroCheck(IrOpcode::kCheckedFloat64ToInt32, Type::PlainNumber());
}
TEST(Nops) {
RepresentationChangerTester r;
......
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