Skip to content

Commit 6e54ea6

Browse files
alexmarkovCommit Queue
alexmarkov
authored and
Commit Queue
committed
[vm/compiler] Merge shift instructions to binary instructions
Uint32 shift changed from (uint32, int64) -> uint32 to (uint32, uint32) -> uint32. TEST=ci Change-Id: Ife1307af72a7c24dd8bd1fa00e90c8e7b903a405 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/396043 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent 246050a commit 6e54ea6

File tree

13 files changed

+311
-738
lines changed

13 files changed

+311
-738
lines changed

‎runtime/vm/compiler/aot/aot_call_specializer.cc‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -475,8 +475,8 @@ Definition* AotCallSpecializer::TryOptimizeDivisionOperation(
475475
InsertBefore(instr, sign_bit_position, /*env=*/nullptr,
476476
FlowGraph::kValue);
477477
auto* const sign_bit_extended = new (Z)
478-
ShiftInt64OpInstr(Token::kSHR, left_value,
479-
new (Z) Value(sign_bit_position), DeoptId::kNone);
478+
BinaryInt64OpInstr(Token::kSHR, left_value,
479+
new (Z) Value(sign_bit_position), DeoptId::kNone);
480480
InsertBefore(instr, sign_bit_extended, /*env=*/nullptr,
481481
FlowGraph::kValue);
482482
auto* rounding_adjustment = unboxed_constant(magnitude - 1);
@@ -496,8 +496,8 @@ Definition* AotCallSpecializer::TryOptimizeDivisionOperation(
496496
unboxed_constant(Utils::ShiftForPowerOfTwo(magnitude));
497497
InsertBefore(instr, right_definition, /*env=*/nullptr, FlowGraph::kValue);
498498
right_value = new (Z) Value(right_definition);
499-
result = new (Z) ShiftInt64OpInstr(Token::kSHR, left_value, right_value,
500-
DeoptId::kNone);
499+
result = new (Z) BinaryInt64OpInstr(Token::kSHR, left_value, right_value,
500+
DeoptId::kNone);
501501
} else {
502502
ASSERT_EQUAL(magnitude, 1);
503503
// No division needed, just redefine the value.
@@ -593,18 +593,10 @@ bool AotCallSpecializer::TryOptimizeIntegerOperation(TemplateDartCall<0>* instr,
593593
case Token::kSUB:
594594
FALL_THROUGH;
595595
case Token::kMUL: {
596-
if (op_kind == Token::kSHL || op_kind == Token::kSHR ||
597-
op_kind == Token::kUSHR) {
598-
left_value = PrepareStaticOpInput(left_value, kMintCid, instr);
599-
right_value = PrepareStaticOpInput(right_value, kMintCid, instr);
600-
replacement = new (Z) ShiftInt64OpInstr(op_kind, left_value,
601-
right_value, DeoptId::kNone);
602-
} else {
603-
left_value = PrepareStaticOpInput(left_value, kMintCid, instr);
604-
right_value = PrepareStaticOpInput(right_value, kMintCid, instr);
605-
replacement = new (Z) BinaryInt64OpInstr(op_kind, left_value,
606-
right_value, DeoptId::kNone);
607-
}
596+
left_value = PrepareStaticOpInput(left_value, kMintCid, instr);
597+
right_value = PrepareStaticOpInput(right_value, kMintCid, instr);
598+
replacement = new (Z) BinaryInt64OpInstr(op_kind, left_value,
599+
right_value, DeoptId::kNone);
608600
break;
609601
}
610602

‎runtime/vm/compiler/backend/constant_propagator.cc‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,14 +1193,6 @@ void ConstantPropagator::VisitBinaryInt64Op(BinaryInt64OpInstr* instr) {
11931193
VisitBinaryIntegerOp(instr);
11941194
}
11951195

1196-
void ConstantPropagator::VisitShiftInt64Op(ShiftInt64OpInstr* instr) {
1197-
VisitBinaryIntegerOp(instr);
1198-
}
1199-
1200-
void ConstantPropagator::VisitShiftUint32Op(ShiftUint32OpInstr* instr) {
1201-
VisitBinaryIntegerOp(instr);
1202-
}
1203-
12041196
void ConstantPropagator::VisitBoxInt64(BoxInt64Instr* instr) {
12051197
VisitBox(instr);
12061198
}

‎runtime/vm/compiler/backend/il.cc‎

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2078,10 +2078,6 @@ bool BinarySmiOpInstr::ComputeCanDeoptimize() const {
20782078
}
20792079
}
20802080

2081-
bool ShiftIntegerOpInstr::IsShiftCountInRange(int64_t max) const {
2082-
return RangeUtils::IsWithin(shift_range(), 0, max);
2083-
}
2084-
20852081
bool BinaryIntegerOpInstr::RightIsNonZero() const {
20862082
if (right()->BindsToConstant()) {
20872083
const auto& constant = right()->BoundConstant();
@@ -2091,6 +2087,15 @@ bool BinaryIntegerOpInstr::RightIsNonZero() const {
20912087
return !RangeUtils::CanBeZero(right()->definition()->range());
20922088
}
20932089

2090+
bool BinaryIntegerOpInstr::RightIsPositive() const {
2091+
if (right()->BindsToConstant()) {
2092+
const auto& constant = right()->BoundConstant();
2093+
if (!constant.IsInteger()) return false;
2094+
return Integer::Cast(constant).Value() > 0;
2095+
}
2096+
return RangeUtils::IsPositive(right()->definition()->range());
2097+
}
2098+
20942099
bool BinaryIntegerOpInstr::RightIsPowerOfTwoConstant() const {
20952100
if (!right()->BindsToConstant()) return false;
20962101
const Object& constant = right()->BoundConstant();
@@ -2100,6 +2105,16 @@ bool BinaryIntegerOpInstr::RightIsPowerOfTwoConstant() const {
21002105
return Utils::IsPowerOfTwo(Utils::Abs(int_value));
21012106
}
21022107

2108+
bool BinaryIntegerOpInstr::IsShiftCountInRange(int64_t max) const {
2109+
if (right()->BindsToConstant()) {
2110+
const auto& constant = right()->BoundConstant();
2111+
if (!constant.IsInteger()) return false;
2112+
const int64_t value = Integer::Cast(constant).Value();
2113+
return (0 <= value) && (value <= max);
2114+
}
2115+
return RangeUtils::IsWithin(right()->definition()->range(), 0, max);
2116+
}
2117+
21032118
static intptr_t RepresentationBits(Representation r) {
21042119
switch (r) {
21052120
case kTagged:
@@ -2283,21 +2298,10 @@ BinaryIntegerOpInstr* BinaryIntegerOpInstr::Make(Representation representation,
22832298
op = new BinaryInt32OpInstr(op_kind, left, right, deopt_id);
22842299
break;
22852300
case kUnboxedUint32:
2286-
if ((op_kind == Token::kSHL) || (op_kind == Token::kSHR) ||
2287-
(op_kind == Token::kUSHR)) {
2288-
op =
2289-
new ShiftUint32OpInstr(op_kind, left, right, deopt_id, right_range);
2290-
} else {
2291-
op = new BinaryUint32OpInstr(op_kind, left, right, deopt_id);
2292-
}
2301+
op = new BinaryUint32OpInstr(op_kind, left, right, deopt_id);
22932302
break;
22942303
case kUnboxedInt64:
2295-
if ((op_kind == Token::kSHL) || (op_kind == Token::kSHR) ||
2296-
(op_kind == Token::kUSHR)) {
2297-
op = new ShiftInt64OpInstr(op_kind, left, right, deopt_id, right_range);
2298-
} else {
2299-
op = new BinaryInt64OpInstr(op_kind, left, right, deopt_id);
2300-
}
2304+
op = new BinaryInt64OpInstr(op_kind, left, right, deopt_id);
23012305
break;
23022306
default:
23032307
UNREACHABLE();
@@ -2420,19 +2424,12 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
24202424
} else if ((rhs > 0) && Utils::IsPowerOfTwo(rhs)) {
24212425
const int64_t shift_amount = Utils::ShiftForPowerOfTwo(rhs);
24222426
ConstantInstr* constant_shift_amount = flow_graph->GetConstant(
2423-
Smi::Handle(Smi::New(shift_amount)), kUnboxedInt64);
2427+
Smi::Handle(Smi::New(shift_amount)), representation());
24242428
BinaryIntegerOpInstr* shift = BinaryIntegerOpInstr::Make(
24252429
representation(), Token::kSHL, left()->CopyWithType(),
24262430
new Value(constant_shift_amount), GetDeoptId(), can_overflow(),
24272431
is_truncating(), range());
24282432
if (shift != nullptr) {
2429-
// Assign a range to the shift factor, just in case range
2430-
// analysis no longer runs after this rewriting.
2431-
if (auto shift_with_range = shift->AsShiftIntegerOp()) {
2432-
shift_with_range->set_shift_range(
2433-
new Range(RangeBoundary::FromConstant(shift_amount),
2434-
RangeBoundary::FromConstant(shift_amount)));
2435-
}
24362433
if (!MayThrow()) {
24372434
ASSERT(!shift->MayThrow());
24382435
}

‎runtime/vm/compiler/backend/il.h‎

Lines changed: 36 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,6 @@ struct InstrAttrs {
502502
M(UnboxInt64, kNoGC) \
503503
M(CaseInsensitiveCompare, kNoGC) \
504504
M(BinaryInt64Op, kNoGC) \
505-
M(ShiftInt64Op, kNoGC) \
506505
M(UnaryInt64Op, kNoGC) \
507506
M(CheckArrayBound, kNoGC) \
508507
M(GenericCheckBound, kNoGC) \
@@ -527,7 +526,6 @@ struct InstrAttrs {
527526
M(UnboxLane, kNoGC) \
528527
M(BoxLanes, _) \
529528
M(BinaryUint32Op, kNoGC) \
530-
M(ShiftUint32Op, kNoGC) \
531529
M(UnaryUint32Op, kNoGC) \
532530
M(BoxUint32, _) \
533531
M(UnboxUint32, kNoGC) \
@@ -564,7 +562,6 @@ struct InstrAttrs {
564562
M(Condition, _) \
565563
M(InstanceCallBase, _) \
566564
M(ReturnBase, _) \
567-
M(ShiftIntegerOp, _) \
568565
M(UnaryIntegerOp, _) \
569566
M(UnboxInteger, _)
570567

@@ -8737,7 +8734,7 @@ class UnboxInt64Instr : public UnboxIntegerInstr {
87378734

87388735
bool Definition::IsInt64Definition() {
87398736
return (Type()->ToCid() == kMintCid) || IsBinaryInt64Op() ||
8740-
IsUnaryInt64Op() || IsShiftInt64Op() || IsBoxInt64() || IsUnboxInt64();
8737+
IsUnaryInt64Op() || IsBoxInt64() || IsUnboxInt64();
87418738
}
87428739

87438740
// Calls into the runtime and performs a case-insensitive comparison of the
@@ -9234,6 +9231,9 @@ class BinaryIntegerOpInstr : public TemplateDefinition<2, NoThrow, Pure> {
92349231
// that does not include the possibility of being zero.
92359232
bool RightIsNonZero() const;
92369233

9234+
// Returns true if rhs operand is positive.
9235+
bool RightIsPositive() const;
9236+
92379237
// Returns true if right is a non-zero Smi constant which absolute value is
92389238
// a power of two.
92399239
bool RightIsPowerOfTwoConstant() const;
@@ -9267,6 +9267,12 @@ class BinaryIntegerOpInstr : public TemplateDefinition<2, NoThrow, Pure> {
92679267
const Range* right_range,
92689268
Range* range);
92699269

9270+
static constexpr intptr_t kShiftCountLimit = 63;
9271+
9272+
// Returns true if the shift amount (right operand) is guaranteed to be in
9273+
// [0..max] range.
9274+
bool IsShiftCountInRange(int64_t max = kShiftCountLimit) const;
9275+
92709276
private:
92719277
Definition* CreateConstantResult(FlowGraph* graph, const Integer& result);
92729278

@@ -9388,6 +9394,9 @@ class BinaryUint32OpInstr : public BinaryIntegerOpInstr {
93889394
case Token::kBIT_AND:
93899395
case Token::kBIT_OR:
93909396
case Token::kBIT_XOR:
9397+
case Token::kSHL:
9398+
case Token::kSHR:
9399+
case Token::kUSHR:
93919400
return true;
93929401
default:
93939402
return false;
@@ -9399,6 +9408,10 @@ class BinaryUint32OpInstr : public BinaryIntegerOpInstr {
93999408
DECLARE_EMPTY_SERIALIZATION(BinaryUint32OpInstr, BinaryIntegerOpInstr)
94009409

94019410
private:
9411+
static constexpr intptr_t kUint32ShiftCountLimit = 31;
9412+
9413+
void EmitShiftUint32(FlowGraphCompiler* compiler);
9414+
94029415
DISALLOW_COPY_AND_ASSIGN(BinaryUint32OpInstr);
94039416
};
94049417

@@ -9417,9 +9430,24 @@ class BinaryInt64OpInstr : public BinaryIntegerOpInstr {
94179430
return false;
94189431
}
94199432

9433+
virtual bool ComputeCanDeoptimizeAfterCall() const {
9434+
return ((op_kind() == Token::kSHL) || (op_kind() == Token::kSHR) ||
9435+
(op_kind() == Token::kUSHR)) &&
9436+
!CompilerState::Current().is_aot();
9437+
}
9438+
94209439
virtual bool MayThrow() const {
9421-
return (op_kind() == Token::kMOD || op_kind() == Token::kTRUNCDIV) &&
9422-
!RightIsNonZero();
9440+
switch (op_kind()) {
9441+
case Token::kSHL:
9442+
case Token::kSHR:
9443+
case Token::kUSHR:
9444+
return !IsShiftCountInRange();
9445+
case Token::kMOD:
9446+
case Token::kTRUNCDIV:
9447+
return !RightIsNonZero();
9448+
default:
9449+
return false;
9450+
}
94239451
}
94249452

94259453
virtual Representation representation() const { return kUnboxedInt64; }
@@ -9434,118 +9462,9 @@ class BinaryInt64OpInstr : public BinaryIntegerOpInstr {
94349462
DECLARE_EMPTY_SERIALIZATION(BinaryInt64OpInstr, BinaryIntegerOpInstr)
94359463

94369464
private:
9437-
DISALLOW_COPY_AND_ASSIGN(BinaryInt64OpInstr);
9438-
};
9439-
9440-
// Base class for integer shift operations.
9441-
class ShiftIntegerOpInstr : public BinaryIntegerOpInstr {
9442-
public:
9443-
ShiftIntegerOpInstr(Token::Kind op_kind,
9444-
Value* left,
9445-
Value* right,
9446-
intptr_t deopt_id,
9447-
// Provided by BinaryIntegerOpInstr::Make for constant RHS
9448-
Range* right_range = nullptr)
9449-
: BinaryIntegerOpInstr(op_kind, left, right, deopt_id),
9450-
shift_range_(right_range) {
9451-
ASSERT((op_kind == Token::kSHL) || (op_kind == Token::kSHR) ||
9452-
(op_kind == Token::kUSHR));
9453-
mark_truncating();
9454-
}
9455-
9456-
Range* shift_range() const { return shift_range_; }
9457-
9458-
// Set the range directly (takes ownership).
9459-
void set_shift_range(Range* shift_range) { shift_range_ = shift_range; }
9460-
9461-
virtual void InferRange(RangeAnalysis* analysis, Range* range);
9462-
9463-
DECLARE_ABSTRACT_INSTRUCTION(ShiftIntegerOp)
9464-
9465-
#define FIELD_LIST(F) F(Range*, shift_range_)
9466-
9467-
DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(ShiftIntegerOpInstr,
9468-
BinaryIntegerOpInstr,
9469-
FIELD_LIST)
9470-
#undef FIELD_LIST
9471-
9472-
protected:
9473-
static constexpr intptr_t kShiftCountLimit = 63;
9474-
9475-
// Returns true if the shift amount is guaranteed to be in
9476-
// [0..max] range.
9477-
bool IsShiftCountInRange(int64_t max = kShiftCountLimit) const;
9478-
9479-
private:
9480-
DISALLOW_COPY_AND_ASSIGN(ShiftIntegerOpInstr);
9481-
};
9482-
9483-
// Non-speculative int64 shift. Takes 2 unboxed int64.
9484-
// Throws if right operand is negative.
9485-
class ShiftInt64OpInstr : public ShiftIntegerOpInstr {
9486-
public:
9487-
ShiftInt64OpInstr(Token::Kind op_kind,
9488-
Value* left,
9489-
Value* right,
9490-
intptr_t deopt_id,
9491-
Range* right_range = nullptr)
9492-
: ShiftIntegerOpInstr(op_kind, left, right, deopt_id, right_range) {}
9493-
9494-
virtual bool ComputeCanDeoptimize() const { return false; }
9495-
virtual bool ComputeCanDeoptimizeAfterCall() const {
9496-
return !CompilerState::Current().is_aot();
9497-
}
9498-
virtual bool MayThrow() const { return !IsShiftCountInRange(); }
9499-
9500-
virtual Representation representation() const { return kUnboxedInt64; }
9501-
9502-
virtual Representation RequiredInputRepresentation(intptr_t idx) const {
9503-
ASSERT((idx == 0) || (idx == 1));
9504-
return kUnboxedInt64;
9505-
}
9506-
9507-
DECLARE_INSTRUCTION(ShiftInt64Op)
9508-
9509-
DECLARE_EMPTY_SERIALIZATION(ShiftInt64OpInstr, ShiftIntegerOpInstr)
9510-
9511-
private:
9512-
DISALLOW_COPY_AND_ASSIGN(ShiftInt64OpInstr);
9513-
};
9514-
9515-
// Non-speculative uint32 shift. Takes unboxed uint32 and unboxed int64.
9516-
// Throws if right operand is negative.
9517-
class ShiftUint32OpInstr : public ShiftIntegerOpInstr {
9518-
public:
9519-
ShiftUint32OpInstr(Token::Kind op_kind,
9520-
Value* left,
9521-
Value* right,
9522-
intptr_t deopt_id,
9523-
Range* right_range = nullptr)
9524-
: ShiftIntegerOpInstr(op_kind, left, right, deopt_id, right_range) {}
9525-
9526-
virtual bool ComputeCanDeoptimize() const { return false; }
9527-
virtual bool ComputeCanDeoptimizeAfterCall() const {
9528-
return !CompilerState::Current().is_aot();
9529-
}
9530-
virtual bool MayThrow() const {
9531-
return !IsShiftCountInRange(kUint32ShiftCountLimit);
9532-
}
9533-
9534-
virtual Representation representation() const { return kUnboxedUint32; }
9535-
9536-
virtual Representation RequiredInputRepresentation(intptr_t idx) const {
9537-
ASSERT((idx == 0) || (idx == 1));
9538-
return (idx == 0) ? kUnboxedUint32 : kUnboxedInt64;
9539-
}
9540-
9541-
DECLARE_INSTRUCTION(ShiftUint32Op)
9542-
9543-
DECLARE_EMPTY_SERIALIZATION(ShiftUint32OpInstr, ShiftIntegerOpInstr)
9544-
9545-
private:
9546-
static constexpr intptr_t kUint32ShiftCountLimit = 31;
9465+
void EmitShiftInt64(FlowGraphCompiler* compiler);
95479466

9548-
DISALLOW_COPY_AND_ASSIGN(ShiftUint32OpInstr);
9467+
DISALLOW_COPY_AND_ASSIGN(BinaryInt64OpInstr);
95499468
};
95509469

95519470
class UnaryDoubleOpInstr : public TemplateDefinition<1, NoThrow, Pure> {

0 commit comments

Comments
 (0)