[vm/compiler] Correct representation of shift rhs and speculative mode of BinaryUint32OpInstr

Speculative and non-speculative shift instructions have different
representations of RHS input, so canonicalization of multiplication
into a shift should account for that.

Also, mark BinaryUint32Op instruction as always non-speculative -
it never needs to speculate  as it is inserted by range analysis
after it can prove that it is valid to perform Uint32 op.
This prevents insertion of a speculative shift instruction in AOT
when BinaryUint32Op multiplication is canonicalized.

Fixes https://github.com/dart-lang/sdk/issues/55003
TEST=vm/dart/regress_55003_test

Change-Id: Ib1805ce2dc0e6f514e6f3b4a09be08808e75048b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/355820
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This commit is contained in:
Alexander Markov 2024-03-08 15:35:20 +00:00 committed by Commit Queue
parent 261e16e822
commit 97369d0903
3 changed files with 42 additions and 10 deletions

View file

@ -2,16 +2,32 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
int var64 = -1;
num var68 = 2;
// Verifies that compiler correctly chooses representation and
// doesn't crash when canonicalizing multiplication to a shift
// after the final SelectRepresentations pass.
// Regression test for https://github.com/dart-lang/sdk/issues/55003.
@pragma("vm:always-consider-inlining")
Set<bool>? foo0_1() {
return <bool>{
(8 <= (((var64 + -90) * 9223372034707292160) % var68)),
};
import 'package:expect/expect.dart';
int one = int.parse('1');
@pragma('vm:never-inline')
void test1() {
// Truncation of 0xaabbccdd00000004 to uint32 is inserted
// by SelectRepresentations_Final. After that, canonicalization
// replaces BinaryUint32Op multiplication with a shift.
Expect.equals(4, (one * 0xaabbccdd00000004) % 8);
}
@pragma('vm:never-inline')
void test2() {
// Truncation of 0xaabbccdd00000000 to uint32 is inserted
// by SelectRepresentations_Final. After that, canonicalization
// replaces outer BinaryInt64Op multiplication with a shift.
Expect.equals(4, one * (((one * 0xaabbccdd00000000) % 8) + 4));
}
main() {
foo0_1();
test1();
test2();
}

View file

@ -2434,6 +2434,13 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
}
}
if (IsBinaryUint32Op() && HasUnmatchedInputRepresentations()) {
// Canonicalization may eliminate instruction and loose truncation,
// so it is illegal to canonicalize truncating uint32 instruction
// until all conversions for its inputs are inserted.
return this;
}
switch (op_kind()) {
case Token::kMUL:
if (rhs == 1) {
@ -2442,8 +2449,11 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
return right()->definition();
} else if ((rhs > 0) && Utils::IsPowerOfTwo(rhs)) {
const int64_t shift_amount = Utils::ShiftForPowerOfTwo(rhs);
const Representation shift_amount_rep =
(SpeculativeModeOfInputs() == kNotSpeculative) ? kUnboxedInt64
: kTagged;
ConstantInstr* constant_shift_amount = flow_graph->GetConstant(
Smi::Handle(Smi::New(shift_amount)), kTagged);
Smi::Handle(Smi::New(shift_amount)), shift_amount_rep);
BinaryIntegerOpInstr* shift = BinaryIntegerOpInstr::Make(
representation(), Token::kSHL, left()->CopyWithType(),
new Value(constant_shift_amount), GetDeoptId(), can_overflow(),

View file

@ -9285,6 +9285,10 @@ class BinaryUint32OpInstr : public BinaryIntegerOpInstr {
return kUnboxedUint32;
}
virtual SpeculativeMode SpeculativeModeOfInput(intptr_t index) const {
return kNotSpeculative;
}
static bool IsSupported(Token::Kind op_kind) {
switch (op_kind) {
case Token::kADD:
@ -9479,7 +9483,9 @@ class ShiftUint32OpInstr : public ShiftIntegerOpInstr {
return kNotSpeculative;
}
virtual bool ComputeCanDeoptimize() const { return false; }
virtual bool MayThrow() const { return true; }
virtual bool MayThrow() const {
return !IsShiftCountInRange(kUint32ShiftCountLimit);
}
virtual Representation representation() const { return kUnboxedUint32; }