From dca3b16e9068212d161d86401dc73f4f541a6765 Mon Sep 17 00:00:00 2001 From: Darius M Date: Thu, 10 Mar 2022 13:11:15 +0100 Subject: [PATCH] Reland [compiler] Improve code generated for patterns like "x >> 1 == 0" This is a reland of 2dc40370497c9b8f88740b0d6069a314da3fde1b Original change's description: > [compiler] Improve code generated for patterns like "x >> 1 == 0" > > Change-Id: I79575ba61a3bdea93468f48d66a3cb3edd0e1442 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3506504 > Reviewed-by: Tobias Tebbi > Commit-Queue: Darius Mercadier > Cr-Commit-Position: refs/heads/main@{#79419} Change-Id: Iad111f8d4bb40a295903dd67f66c8ecd9c4eadd9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3514072 Reviewed-by: Tobias Tebbi Commit-Queue: Darius Mercadier Cr-Commit-Position: refs/heads/main@{#79443} --- src/compiler/machine-operator-reducer.cc | 66 +++++++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/src/compiler/machine-operator-reducer.cc b/src/compiler/machine-operator-reducer.cc index 942e7a17f1..61b9df4616 100644 --- a/src/compiler/machine-operator-reducer.cc +++ b/src/compiler/machine-operator-reducer.cc @@ -1348,13 +1348,33 @@ Reduction MachineOperatorReducer::ReduceProjection(size_t index, Node* node) { return NoChange(); } +namespace { + +// Returns true if "value << shift >> shift == value". This can be interpreted +// as "left shifting |value| by |shift| doesn't shift away significant bits". +// Or, equivalently, "left shifting |value| by |shift| doesn't have signed +// overflow". +bool CanRevertLeftShiftWithRightShift(int32_t value, int32_t shift) { + if (shift < 0 || shift >= 32) { + // This shift would be UB in C++ + return false; + } + if (static_cast(static_cast(value) << shift) >> shift != + static_cast(value)) { + return false; + } + return true; +} + +} // namespace + Reduction MachineOperatorReducer::ReduceWord32Comparisons(Node* node) { DCHECK(node->opcode() == IrOpcode::kInt32LessThan || node->opcode() == IrOpcode::kInt32LessThanOrEqual || node->opcode() == IrOpcode::kUint32LessThan || node->opcode() == IrOpcode::kUint32LessThanOrEqual); Int32BinopMatcher m(node); - // (x >>> K) < (y >>> K) => x < y if only zeros shifted out + // (x >> K) < (y >> K) => x < y if only zeros shifted out if (m.left().op() == machine()->Word32SarShiftOutZeros() && m.right().op() == machine()->Word32SarShiftOutZeros()) { Int32BinopMatcher mleft(m.left().node()); @@ -1366,6 +1386,36 @@ Reduction MachineOperatorReducer::ReduceWord32Comparisons(Node* node) { return Changed(node); } } + // Simplifying (x >> n) <= k into x <= (k << n), with "k << n" being + // computed here at compile time. + if (m.right().HasResolvedValue() && + m.left().op() == machine()->Word32SarShiftOutZeros()) { + uint32_t right = m.right().ResolvedValue(); + Int32BinopMatcher mleft(m.left().node()); + if (mleft.right().HasResolvedValue()) { + auto shift = mleft.right().ResolvedValue(); + if (CanRevertLeftShiftWithRightShift(right, shift)) { + node->ReplaceInput(0, mleft.left().node()); + node->ReplaceInput(1, Int32Constant(right << shift)); + return Changed(node); + } + } + } + // Simplifying k <= (x >> n) into (k << n) <= x, with "k << n" being + // computed here at compile time. + if (m.left().HasResolvedValue() && + m.right().op() == machine()->Word32SarShiftOutZeros()) { + uint32_t left = m.left().ResolvedValue(); + Int32BinopMatcher mright(m.right().node()); + if (mright.right().HasResolvedValue()) { + auto shift = mright.right().ResolvedValue(); + if (CanRevertLeftShiftWithRightShift(left, shift)) { + node->ReplaceInput(0, Int32Constant(left << shift)); + node->ReplaceInput(1, mright.left().node()); + return Changed(node); + } + } + } return NoChange(); } @@ -1405,7 +1455,7 @@ Reduction MachineOperatorReducer::ReduceWord64Comparisons(Node* node) { return Changed(node).FollowedBy(Reduce(node)); } - // (x >>> K) < (y >>> K) => x < y if only zeros shifted out + // (x >> K) < (y >> K) => x < y if only zeros shifted out // This is useful for Smi untagging, which results in such a shift. if (m.left().op() == machine()->Word64SarShiftOutZeros() && m.right().op() == machine()->Word64SarShiftOutZeros()) { @@ -2202,6 +2252,18 @@ MachineOperatorReducer::ReduceWord32EqualForConstantRhs(Node* lhs, } } } + // Replaces (x >> n) == k with x == k << n, with "k << n" being computed + // here at compile time. + if (lhs->op() == machine()->Word32SarShiftOutZeros() && + lhs->UseCount() == 1) { + typename WordNAdapter::UintNBinopMatcher mshift(lhs); + if (mshift.right().HasResolvedValue()) { + int32_t shift = static_cast(mshift.right().ResolvedValue()); + if (CanRevertLeftShiftWithRightShift(rhs, shift)) { + return std::make_pair(mshift.left().node(), rhs << shift); + } + } + } return {}; }