Commit 35974e2d authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[turbofan] Improve CheckedInt32Mod lowering.

The CheckedInt32Mod lowering in the EffectControlLinearizer wasn't
playing well with subsequent optimizations in the MachineOperatorReducer
especially due to the use of Int32Mod, which introduces another (floating)
diamond in the MachineOperatorReducer. Switching to Uint32Mod and explicit
sign handling fixes the problem, plus we also do the mask trick in the
case where the left hand side is negative now.

With this change the performance on the benchmark mentioned in the bug
report goes from

  console.timeEnd: binary, 1872.346000
  console.timeEnd: modulo, 5967.464000
  console.timeEnd: binary, 6006.789000
  console.timeEnd: modulo, 6293.496000
  console.timeEnd: binary, 5969.264000
  console.timeEnd: modulo, 6291.874000

to

  console.timeEnd: binary, 1876.464000
  console.timeEnd: modulo, 5846.643000
  console.timeEnd: binary, 5962.545000
  console.timeEnd: modulo, 5972.639000
  console.timeEnd: binary, 5958.221000
  console.timeEnd: modulo, 5973.171000

so even the peak performance of the modulus is now mostly the same as
the binary bitwise and.

Bug: v8:8069
Change-Id: Iaf3828fc0f6c53352367e8bf6c42534f8b13bfb3
Reviewed-on: https://chromium-review.googlesource.com/1180971Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55211}
parent 3b8fd36b
......@@ -1631,6 +1631,32 @@ Node* EffectControlLinearizer::LowerCheckedInt32Div(Node* node,
return value;
}
Node* EffectControlLinearizer::BuildUint32Mod(Node* lhs, Node* rhs) {
auto if_rhs_power_of_two = __ MakeLabel();
auto done = __ MakeLabel(MachineRepresentation::kWord32);
// Compute the mask for the {rhs}.
Node* one = __ Int32Constant(1);
Node* msk = __ Int32Sub(rhs, one);
// Check if the {rhs} is a power of two.
__ GotoIf(__ Word32Equal(__ Word32And(rhs, msk), __ Int32Constant(0)),
&if_rhs_power_of_two);
{
// The {rhs} is not a power of two, do a generic Uint32Mod.
__ Goto(&done, __ Uint32Mod(lhs, rhs));
}
__ Bind(&if_rhs_power_of_two);
{
// The {rhs} is a power of two, just do a fast bit masking.
__ Goto(&done, __ Word32And(lhs, msk));
}
__ Bind(&done);
return done.PhiAt(0);
}
Node* EffectControlLinearizer::LowerCheckedInt32Mod(Node* node,
Node* frame_state) {
// General case for signed integer modulus, with optimization for (unknown)
......@@ -1639,12 +1665,19 @@ Node* EffectControlLinearizer::LowerCheckedInt32Mod(Node* node,
// if rhs <= 0 then
// rhs = -rhs
// deopt if rhs == 0
// let msk = rhs - 1 in
// if lhs < 0 then
// let res = lhs % rhs in
// deopt if res == 0
// res
// let lhs_abs = -lsh in
// let res = if rhs & msk == 0 then
// lhs_abs & msk
// else
// lhs_abs % rhs in
// if lhs < 0 then
// deopt if res == 0
// -res
// else
// res
// else
// let msk = rhs - 1 in
// if rhs & msk == 0 then
// lhs & msk
// else
......@@ -1655,7 +1688,7 @@ Node* EffectControlLinearizer::LowerCheckedInt32Mod(Node* node,
auto if_rhs_not_positive = __ MakeDeferredLabel();
auto if_lhs_negative = __ MakeDeferredLabel();
auto if_power_of_two = __ MakeLabel();
auto if_rhs_power_of_two = __ MakeLabel();
auto rhs_checked = __ MakeLabel(MachineRepresentation::kWord32);
auto done = __ MakeLabel(MachineRepresentation::kWord32);
......@@ -1673,45 +1706,29 @@ Node* EffectControlLinearizer::LowerCheckedInt32Mod(Node* node,
Node* vtrue0 = __ Int32Sub(zero, rhs);
// Ensure that {rhs} is not zero, otherwise we'd have to return NaN.
Node* check = __ Word32Equal(vtrue0, zero);
__ DeoptimizeIf(DeoptimizeReason::kDivisionByZero, VectorSlotPair(), check,
frame_state);
__ DeoptimizeIf(DeoptimizeReason::kDivisionByZero, VectorSlotPair(),
__ Word32Equal(vtrue0, zero), frame_state);
__ Goto(&rhs_checked, vtrue0);
}
__ Bind(&rhs_checked);
rhs = rhs_checked.PhiAt(0);
// Check if {lhs} is negative.
Node* check1 = __ Int32LessThan(lhs, zero);
__ GotoIf(check1, &if_lhs_negative);
// {lhs} non-negative.
__ GotoIf(__ Int32LessThan(lhs, zero), &if_lhs_negative);
{
Node* one = __ Int32Constant(1);
Node* msk = __ Int32Sub(rhs, one);
// Check if {rhs} minus one is a valid mask.
Node* check2 = __ Word32Equal(__ Word32And(rhs, msk), zero);
__ GotoIf(check2, &if_power_of_two);
// Compute the remainder using the generic {lhs % rhs}.
__ Goto(&done, __ Int32Mod(lhs, rhs));
__ Bind(&if_power_of_two);
// Compute the remainder using {lhs & msk}.
__ Goto(&done, __ Word32And(lhs, msk));
// The {lhs} is a non-negative integer.
__ Goto(&done, BuildUint32Mod(lhs, rhs));
}
__ Bind(&if_lhs_negative);
{
// Compute the remainder using {lhs % msk}.
Node* vtrue1 = __ Int32Mod(lhs, rhs);
// The {lhs} is a negative integer.
Node* res = BuildUint32Mod(__ Int32Sub(zero, lhs), rhs);
// Check if we would have to return -0.
Node* check = __ Word32Equal(vtrue1, zero);
__ DeoptimizeIf(DeoptimizeReason::kMinusZero, VectorSlotPair(), check,
frame_state);
__ Goto(&done, vtrue1);
__ DeoptimizeIf(DeoptimizeReason::kMinusZero, VectorSlotPair(),
__ Word32Equal(res, zero), frame_state);
__ Goto(&done, __ Int32Sub(zero, res));
}
__ Bind(&done);
......@@ -1753,7 +1770,7 @@ Node* EffectControlLinearizer::LowerCheckedUint32Mod(Node* node,
frame_state);
// Perform the actual unsigned integer modulus.
return __ Uint32Mod(lhs, rhs);
return BuildUint32Mod(lhs, rhs);
}
Node* EffectControlLinearizer::LowerCheckedInt32Mul(Node* node,
......
......@@ -181,6 +181,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
Node* BuildReverseBytes(ExternalArrayType type, Node* value);
Node* BuildFloat64RoundDown(Node* value);
Node* BuildFloat64RoundTruncate(Node* input);
Node* BuildUint32Mod(Node* lhs, Node* rhs);
Node* ComputeIntegerHash(Node* value);
Node* LowerStringComparison(Callable const& callable, Node* node);
Node* IsElementsKindGreaterThan(Node* kind, ElementsKind reference_kind);
......
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