Skip to content

Commit 236c3c1

Browse files
committed
aarch64: Fix incorrect encoding of large const values in icmp.
When encoding constants as immediates into an RSE Imm12 instruction we need to take special care to check if the value that we are trying to input does not overflow its type when viewed as a signed value. (i.e. iconst.i8 200) We cannot both put an immediate and sign extend it, so we need to lower it into a separate reg, and emit the sign extend into the instruction. For more details see the [cg_clif bug report](https://github.com/bjorn3/rustc_codegen_cranelift/issues/1184#issuecomment-873214796).
1 parent f2d2f3a commit 236c3c1

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

cranelift/codegen/src/isa/aarch64/lower.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,14 @@ impl NarrowValueMode {
156156
NarrowValueMode::ZeroExtend64 | NarrowValueMode::SignExtend64 => false,
157157
}
158158
}
159+
160+
fn is_signed(&self) -> bool {
161+
match self {
162+
NarrowValueMode::SignExtend32 | NarrowValueMode::SignExtend64 => true,
163+
NarrowValueMode::ZeroExtend32 | NarrowValueMode::ZeroExtend64 => false,
164+
NarrowValueMode::None => false,
165+
}
166+
}
159167
}
160168

161169
/// Emits instruction(s) to generate the given constant value into newly-allocated
@@ -438,7 +446,20 @@ pub(crate) fn put_input_in_rse_imm12<C: LowerCtx<I = Inst>>(
438446
) -> ResultRSEImm12 {
439447
if let Some(imm_value) = input_to_const(ctx, input) {
440448
if let Some(i) = Imm12::maybe_from_u64(imm_value) {
441-
return ResultRSEImm12::Imm12(i);
449+
let out_ty_bits = ty_bits(ctx.output_ty(input.insn, 0));
450+
let ty_signed_max = if out_ty_bits <= 1 {
451+
out_ty_bits as i64
452+
} else {
453+
(1i64 << (out_ty_bits - 1)) - 1
454+
};
455+
let is_negative = i.bits > (ty_signed_max as u16);
456+
457+
// This condition can happen if we matched a value that overflows the output type of
458+
// its `iconst` when viewed as a signed value (i.e. iconst.i8 200).
459+
// When that happens we need to lower as a negative value, which we cannot do here.
460+
if !(narrow_mode.is_signed() && is_negative) {
461+
return ResultRSEImm12::Imm12(i);
462+
}
442463
}
443464
}
444465

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
test run
2+
target aarch64
3+
target s390x
4+
target x86_64 machinst
5+
6+
; This test is also a regression test for aarch64.
7+
; We were not correctly handling the fact that the rhs constant value
8+
; overflows its type when viewed as a signed value, and thus encoding the wrong
9+
; value into the resulting instruction.
10+
function %overflow_rhs_const(i8) -> b1 {
11+
block0(v0: i8):
12+
v1 = iconst.i8 192
13+
v2 = icmp sge v0, v1
14+
return v2
15+
}
16+
; run: %test(49) == true
17+
; run: %test(-65) == false

0 commit comments

Comments
 (0)