Skip to content

Commit

Permalink
[vm/compiler] Merge shift instructions to binary instructions
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
alexmarkov authored and Commit Queue committed Nov 19, 2024
1 parent 246050a commit 6e54ea6
Show file tree
Hide file tree
Showing 13 changed files with 311 additions and 738 deletions.
24 changes: 8 additions & 16 deletions runtime/vm/compiler/aot/aot_call_specializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,8 @@ Definition* AotCallSpecializer::TryOptimizeDivisionOperation(
InsertBefore(instr, sign_bit_position, /*env=*/nullptr,
FlowGraph::kValue);
auto* const sign_bit_extended = new (Z)
ShiftInt64OpInstr(Token::kSHR, left_value,
new (Z) Value(sign_bit_position), DeoptId::kNone);
BinaryInt64OpInstr(Token::kSHR, left_value,
new (Z) Value(sign_bit_position), DeoptId::kNone);
InsertBefore(instr, sign_bit_extended, /*env=*/nullptr,
FlowGraph::kValue);
auto* rounding_adjustment = unboxed_constant(magnitude - 1);
Expand All @@ -496,8 +496,8 @@ Definition* AotCallSpecializer::TryOptimizeDivisionOperation(
unboxed_constant(Utils::ShiftForPowerOfTwo(magnitude));
InsertBefore(instr, right_definition, /*env=*/nullptr, FlowGraph::kValue);
right_value = new (Z) Value(right_definition);
result = new (Z) ShiftInt64OpInstr(Token::kSHR, left_value, right_value,
DeoptId::kNone);
result = new (Z) BinaryInt64OpInstr(Token::kSHR, left_value, right_value,
DeoptId::kNone);
} else {
ASSERT_EQUAL(magnitude, 1);
// No division needed, just redefine the value.
Expand Down Expand Up @@ -593,18 +593,10 @@ bool AotCallSpecializer::TryOptimizeIntegerOperation(TemplateDartCall<0>* instr,
case Token::kSUB:
FALL_THROUGH;
case Token::kMUL: {
if (op_kind == Token::kSHL || op_kind == Token::kSHR ||
op_kind == Token::kUSHR) {
left_value = PrepareStaticOpInput(left_value, kMintCid, instr);
right_value = PrepareStaticOpInput(right_value, kMintCid, instr);
replacement = new (Z) ShiftInt64OpInstr(op_kind, left_value,
right_value, DeoptId::kNone);
} else {
left_value = PrepareStaticOpInput(left_value, kMintCid, instr);
right_value = PrepareStaticOpInput(right_value, kMintCid, instr);
replacement = new (Z) BinaryInt64OpInstr(op_kind, left_value,
right_value, DeoptId::kNone);
}
left_value = PrepareStaticOpInput(left_value, kMintCid, instr);
right_value = PrepareStaticOpInput(right_value, kMintCid, instr);
replacement = new (Z) BinaryInt64OpInstr(op_kind, left_value,
right_value, DeoptId::kNone);
break;
}

Expand Down
8 changes: 0 additions & 8 deletions runtime/vm/compiler/backend/constant_propagator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1193,14 +1193,6 @@ void ConstantPropagator::VisitBinaryInt64Op(BinaryInt64OpInstr* instr) {
VisitBinaryIntegerOp(instr);
}

void ConstantPropagator::VisitShiftInt64Op(ShiftInt64OpInstr* instr) {
VisitBinaryIntegerOp(instr);
}

void ConstantPropagator::VisitShiftUint32Op(ShiftUint32OpInstr* instr) {
VisitBinaryIntegerOp(instr);
}

void ConstantPropagator::VisitBoxInt64(BoxInt64Instr* instr) {
VisitBox(instr);
}
Expand Down
47 changes: 22 additions & 25 deletions runtime/vm/compiler/backend/il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2078,10 +2078,6 @@ bool BinarySmiOpInstr::ComputeCanDeoptimize() const {
}
}

bool ShiftIntegerOpInstr::IsShiftCountInRange(int64_t max) const {
return RangeUtils::IsWithin(shift_range(), 0, max);
}

bool BinaryIntegerOpInstr::RightIsNonZero() const {
if (right()->BindsToConstant()) {
const auto& constant = right()->BoundConstant();
Expand All @@ -2091,6 +2087,15 @@ bool BinaryIntegerOpInstr::RightIsNonZero() const {
return !RangeUtils::CanBeZero(right()->definition()->range());
}

bool BinaryIntegerOpInstr::RightIsPositive() const {
if (right()->BindsToConstant()) {
const auto& constant = right()->BoundConstant();
if (!constant.IsInteger()) return false;
return Integer::Cast(constant).Value() > 0;
}
return RangeUtils::IsPositive(right()->definition()->range());
}

bool BinaryIntegerOpInstr::RightIsPowerOfTwoConstant() const {
if (!right()->BindsToConstant()) return false;
const Object& constant = right()->BoundConstant();
Expand All @@ -2100,6 +2105,16 @@ bool BinaryIntegerOpInstr::RightIsPowerOfTwoConstant() const {
return Utils::IsPowerOfTwo(Utils::Abs(int_value));
}

bool BinaryIntegerOpInstr::IsShiftCountInRange(int64_t max) const {
if (right()->BindsToConstant()) {
const auto& constant = right()->BoundConstant();
if (!constant.IsInteger()) return false;
const int64_t value = Integer::Cast(constant).Value();
return (0 <= value) && (value <= max);
}
return RangeUtils::IsWithin(right()->definition()->range(), 0, max);
}

static intptr_t RepresentationBits(Representation r) {
switch (r) {
case kTagged:
Expand Down Expand Up @@ -2283,21 +2298,10 @@ BinaryIntegerOpInstr* BinaryIntegerOpInstr::Make(Representation representation,
op = new BinaryInt32OpInstr(op_kind, left, right, deopt_id);
break;
case kUnboxedUint32:
if ((op_kind == Token::kSHL) || (op_kind == Token::kSHR) ||
(op_kind == Token::kUSHR)) {
op =
new ShiftUint32OpInstr(op_kind, left, right, deopt_id, right_range);
} else {
op = new BinaryUint32OpInstr(op_kind, left, right, deopt_id);
}
op = new BinaryUint32OpInstr(op_kind, left, right, deopt_id);
break;
case kUnboxedInt64:
if ((op_kind == Token::kSHL) || (op_kind == Token::kSHR) ||
(op_kind == Token::kUSHR)) {
op = new ShiftInt64OpInstr(op_kind, left, right, deopt_id, right_range);
} else {
op = new BinaryInt64OpInstr(op_kind, left, right, deopt_id);
}
op = new BinaryInt64OpInstr(op_kind, left, right, deopt_id);
break;
default:
UNREACHABLE();
Expand Down Expand Up @@ -2420,19 +2424,12 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
} else if ((rhs > 0) && Utils::IsPowerOfTwo(rhs)) {
const int64_t shift_amount = Utils::ShiftForPowerOfTwo(rhs);
ConstantInstr* constant_shift_amount = flow_graph->GetConstant(
Smi::Handle(Smi::New(shift_amount)), kUnboxedInt64);
Smi::Handle(Smi::New(shift_amount)), representation());
BinaryIntegerOpInstr* shift = BinaryIntegerOpInstr::Make(
representation(), Token::kSHL, left()->CopyWithType(),
new Value(constant_shift_amount), GetDeoptId(), can_overflow(),
is_truncating(), range());
if (shift != nullptr) {
// Assign a range to the shift factor, just in case range
// analysis no longer runs after this rewriting.
if (auto shift_with_range = shift->AsShiftIntegerOp()) {
shift_with_range->set_shift_range(
new Range(RangeBoundary::FromConstant(shift_amount),
RangeBoundary::FromConstant(shift_amount)));
}
if (!MayThrow()) {
ASSERT(!shift->MayThrow());
}
Expand Down
153 changes: 36 additions & 117 deletions runtime/vm/compiler/backend/il.h
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,6 @@ struct InstrAttrs {
M(UnboxInt64, kNoGC) \
M(CaseInsensitiveCompare, kNoGC) \
M(BinaryInt64Op, kNoGC) \
M(ShiftInt64Op, kNoGC) \
M(UnaryInt64Op, kNoGC) \
M(CheckArrayBound, kNoGC) \
M(GenericCheckBound, kNoGC) \
Expand All @@ -527,7 +526,6 @@ struct InstrAttrs {
M(UnboxLane, kNoGC) \
M(BoxLanes, _) \
M(BinaryUint32Op, kNoGC) \
M(ShiftUint32Op, kNoGC) \
M(UnaryUint32Op, kNoGC) \
M(BoxUint32, _) \
M(UnboxUint32, kNoGC) \
Expand Down Expand Up @@ -564,7 +562,6 @@ struct InstrAttrs {
M(Condition, _) \
M(InstanceCallBase, _) \
M(ReturnBase, _) \
M(ShiftIntegerOp, _) \
M(UnaryIntegerOp, _) \
M(UnboxInteger, _)

Expand Down Expand Up @@ -8737,7 +8734,7 @@ class UnboxInt64Instr : public UnboxIntegerInstr {

bool Definition::IsInt64Definition() {
return (Type()->ToCid() == kMintCid) || IsBinaryInt64Op() ||
IsUnaryInt64Op() || IsShiftInt64Op() || IsBoxInt64() || IsUnboxInt64();
IsUnaryInt64Op() || IsBoxInt64() || IsUnboxInt64();
}

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

// Returns true if rhs operand is positive.
bool RightIsPositive() const;

// Returns true if right is a non-zero Smi constant which absolute value is
// a power of two.
bool RightIsPowerOfTwoConstant() const;
Expand Down Expand Up @@ -9267,6 +9267,12 @@ class BinaryIntegerOpInstr : public TemplateDefinition<2, NoThrow, Pure> {
const Range* right_range,
Range* range);

static constexpr intptr_t kShiftCountLimit = 63;

// Returns true if the shift amount (right operand) is guaranteed to be in
// [0..max] range.
bool IsShiftCountInRange(int64_t max = kShiftCountLimit) const;

private:
Definition* CreateConstantResult(FlowGraph* graph, const Integer& result);

Expand Down Expand Up @@ -9388,6 +9394,9 @@ class BinaryUint32OpInstr : public BinaryIntegerOpInstr {
case Token::kBIT_AND:
case Token::kBIT_OR:
case Token::kBIT_XOR:
case Token::kSHL:
case Token::kSHR:
case Token::kUSHR:
return true;
default:
return false;
Expand All @@ -9399,6 +9408,10 @@ class BinaryUint32OpInstr : public BinaryIntegerOpInstr {
DECLARE_EMPTY_SERIALIZATION(BinaryUint32OpInstr, BinaryIntegerOpInstr)

private:
static constexpr intptr_t kUint32ShiftCountLimit = 31;

void EmitShiftUint32(FlowGraphCompiler* compiler);

DISALLOW_COPY_AND_ASSIGN(BinaryUint32OpInstr);
};

Expand All @@ -9417,9 +9430,24 @@ class BinaryInt64OpInstr : public BinaryIntegerOpInstr {
return false;
}

virtual bool ComputeCanDeoptimizeAfterCall() const {
return ((op_kind() == Token::kSHL) || (op_kind() == Token::kSHR) ||
(op_kind() == Token::kUSHR)) &&
!CompilerState::Current().is_aot();
}

virtual bool MayThrow() const {
return (op_kind() == Token::kMOD || op_kind() == Token::kTRUNCDIV) &&
!RightIsNonZero();
switch (op_kind()) {
case Token::kSHL:
case Token::kSHR:
case Token::kUSHR:
return !IsShiftCountInRange();
case Token::kMOD:
case Token::kTRUNCDIV:
return !RightIsNonZero();
default:
return false;
}
}

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

private:
DISALLOW_COPY_AND_ASSIGN(BinaryInt64OpInstr);
};

// Base class for integer shift operations.
class ShiftIntegerOpInstr : public BinaryIntegerOpInstr {
public:
ShiftIntegerOpInstr(Token::Kind op_kind,
Value* left,
Value* right,
intptr_t deopt_id,
// Provided by BinaryIntegerOpInstr::Make for constant RHS
Range* right_range = nullptr)
: BinaryIntegerOpInstr(op_kind, left, right, deopt_id),
shift_range_(right_range) {
ASSERT((op_kind == Token::kSHL) || (op_kind == Token::kSHR) ||
(op_kind == Token::kUSHR));
mark_truncating();
}

Range* shift_range() const { return shift_range_; }

// Set the range directly (takes ownership).
void set_shift_range(Range* shift_range) { shift_range_ = shift_range; }

virtual void InferRange(RangeAnalysis* analysis, Range* range);

DECLARE_ABSTRACT_INSTRUCTION(ShiftIntegerOp)

#define FIELD_LIST(F) F(Range*, shift_range_)

DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(ShiftIntegerOpInstr,
BinaryIntegerOpInstr,
FIELD_LIST)
#undef FIELD_LIST

protected:
static constexpr intptr_t kShiftCountLimit = 63;

// Returns true if the shift amount is guaranteed to be in
// [0..max] range.
bool IsShiftCountInRange(int64_t max = kShiftCountLimit) const;

private:
DISALLOW_COPY_AND_ASSIGN(ShiftIntegerOpInstr);
};

// Non-speculative int64 shift. Takes 2 unboxed int64.
// Throws if right operand is negative.
class ShiftInt64OpInstr : public ShiftIntegerOpInstr {
public:
ShiftInt64OpInstr(Token::Kind op_kind,
Value* left,
Value* right,
intptr_t deopt_id,
Range* right_range = nullptr)
: ShiftIntegerOpInstr(op_kind, left, right, deopt_id, right_range) {}

virtual bool ComputeCanDeoptimize() const { return false; }
virtual bool ComputeCanDeoptimizeAfterCall() const {
return !CompilerState::Current().is_aot();
}
virtual bool MayThrow() const { return !IsShiftCountInRange(); }

virtual Representation representation() const { return kUnboxedInt64; }

virtual Representation RequiredInputRepresentation(intptr_t idx) const {
ASSERT((idx == 0) || (idx == 1));
return kUnboxedInt64;
}

DECLARE_INSTRUCTION(ShiftInt64Op)

DECLARE_EMPTY_SERIALIZATION(ShiftInt64OpInstr, ShiftIntegerOpInstr)

private:
DISALLOW_COPY_AND_ASSIGN(ShiftInt64OpInstr);
};

// Non-speculative uint32 shift. Takes unboxed uint32 and unboxed int64.
// Throws if right operand is negative.
class ShiftUint32OpInstr : public ShiftIntegerOpInstr {
public:
ShiftUint32OpInstr(Token::Kind op_kind,
Value* left,
Value* right,
intptr_t deopt_id,
Range* right_range = nullptr)
: ShiftIntegerOpInstr(op_kind, left, right, deopt_id, right_range) {}

virtual bool ComputeCanDeoptimize() const { return false; }
virtual bool ComputeCanDeoptimizeAfterCall() const {
return !CompilerState::Current().is_aot();
}
virtual bool MayThrow() const {
return !IsShiftCountInRange(kUint32ShiftCountLimit);
}

virtual Representation representation() const { return kUnboxedUint32; }

virtual Representation RequiredInputRepresentation(intptr_t idx) const {
ASSERT((idx == 0) || (idx == 1));
return (idx == 0) ? kUnboxedUint32 : kUnboxedInt64;
}

DECLARE_INSTRUCTION(ShiftUint32Op)

DECLARE_EMPTY_SERIALIZATION(ShiftUint32OpInstr, ShiftIntegerOpInstr)

private:
static constexpr intptr_t kUint32ShiftCountLimit = 31;
void EmitShiftInt64(FlowGraphCompiler* compiler);

DISALLOW_COPY_AND_ASSIGN(ShiftUint32OpInstr);
DISALLOW_COPY_AND_ASSIGN(BinaryInt64OpInstr);
};

class UnaryDoubleOpInstr : public TemplateDefinition<1, NoThrow, Pure> {
Expand Down
Loading

0 comments on commit 6e54ea6

Please sign in to comment.