Change in dart/sdk[master]: [vm/compiler] Strength reduce x * 2^n to x << n for n > 1
Vyacheslav Egorov (Gerrit)
Vyacheslav Egorov (Gerrit)
Vyacheslav Egorov would like Aart Bik to review this change.
[vm/compiler] Strength reduce x * 2^n to x << n for n > 1
For some unclear reason we only handled n = 0 and n = 1 before.
Revealed by looking at disassembly from https://github.com/flutter/flutter/issues/19666
Change-Id: If82b3bee7b28357fcc5e0559188ace91a94d1b49
---
M runtime/vm/compiler/backend/il.cc
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
index 2b5574f..709446d 100644
--- a/runtime/vm/compiler/backend/il.cc
+++ b/runtime/vm/compiler/backend/il.cc
@@ -2414,12 +2414,12 @@
return left()->definition();
} else if (rhs == 0) {
return right()->definition();
- } else if (rhs == 2) {
- ConstantInstr* constant_1 =
- flow_graph->GetConstant(Smi::Handle(Smi::New(1)));
+ } else if (Utils::IsPowerOfTwo(static_cast<uword>(rhs))) {
+ ConstantInstr* shift_amount = flow_graph->GetConstant(Smi::Handle(
+ Smi::New(Utils::ShiftForPowerOfTwo(static_cast<uword>(rhs)))));
BinaryIntegerOpInstr* shift = BinaryIntegerOpInstr::Make(
representation(), Token::kSHL, left()->CopyWithType(),
- new Value(constant_1), GetDeoptId(), can_overflow(),
+ new Value(shift_amount), GetDeoptId(), can_overflow(),
is_truncating(), range(), speculative_mode());
if (shift != NULL) {
flow_graph->InsertBefore(this, shift, env(), FlowGraph::kValue);
@@ -3213,7 +3213,6 @@
return true;
}
- // Recognize
if (BindsToGivenConstant(mask_op->left(), value) ||
BindsToGivenConstant(mask_op->right(), value)) {
// Recognized (a & 2^n) == 2^n pattern. It's equivalent to (a & 2^n) != 0
To view, visit change 83420. To unsubscribe, or for help writing mail filters, visit settings.
Aart Bik (Gerrit)
1 comment:
File runtime/vm/compiler/backend/il.cc:
Patch Set #1, Line 2419:
Smi::New(Utils::ShiftForPowerOfTwo(static_cast<uword>(rhs)))));how about adding some tests to this CL?
To view, visit change 83420. To unsubscribe, or for help writing mail filters, visit settings.
Stephen Adams (Gerrit)
"For some unclear reason we only handled n = 0 and n = 1 before"
Might be a hold-over from unbounded int - on some architectures (e.g. x64) you can't tell if the shift overflowed for shifts by more than 1.
Now that you don't care about overflow, on x64 you can also use LEA for x*M+C for M∈{2,3,4,5,8,9}, with the added benefit of targeting a different register.
Aart Bik (Gerrit)
1 comment:
Patch Set #1, Line 2419:
Smi::New(Utils::ShiftForPowerOfTwo(static_cast<uword>(rhs)))));how about adding some tests to this CL?
Slava, are you working on this?
To view, visit change 83420. To unsubscribe, or for help writing mail filters, visit settings.
Vyacheslav Egorov (Gerrit)
1 comment:
File runtime/vm/compiler/backend/il.cc:
Patch Set #1, Line 2419:
Smi::New(Utils::ShiftForPowerOfTwo(static_cast<uword>(rhs)))));
Slava, are you working on this?
Yep. It's the next CL in the list of CLs I am going to get to :)
To view, visit change 83420. To unsubscribe, or for help writing mail filters, visit settings.
Slava Egorov (Gerrit)
Slava Egorov abandoned this change.
To view, visit change 83420. To unsubscribe, or for help writing mail filters, visit settings.