Merge pull request #3056 from afonso360/aarch64-fix-overflow-imm

aarch64: Fix incorrect encoding of large const values in icmp.

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

@@ -24,6 +24,7 @@ use crate::data_value::DataValue;
 use log::{debug, trace};
 use regalloc::{Reg, Writable};
 use smallvec::SmallVec;
+use std::cmp;
 
 //============================================================================
 // Result enum types.
@@ -156,6 +157,14 @@ impl NarrowValueMode {
             NarrowValueMode::ZeroExtend64 | NarrowValueMode::SignExtend64 => false,
         }
     }
+
+    fn is_signed(&self) -> bool {
+        match self {
+            NarrowValueMode::SignExtend32 | NarrowValueMode::SignExtend64 => true,
+            NarrowValueMode::ZeroExtend32 | NarrowValueMode::ZeroExtend64 => false,
+            NarrowValueMode::None => false,
+        }
+    }
 }
 
 /// Emits instruction(s) to generate the given constant value into newly-allocated
@@ -438,9 +447,17 @@ pub(crate) fn put_input_in_rse_imm12<C: LowerCtx<I = Inst>>(
 ) -> ResultRSEImm12 {
     if let Some(imm_value) = input_to_const(ctx, input) {
         if let Some(i) = Imm12::maybe_from_u64(imm_value) {
+            let out_ty_bits = ty_bits(ctx.input_ty(input.insn, input.input));
+            let is_negative = (i.bits as u64) & (1 << (cmp::max(out_ty_bits, 1) - 1)) != 0;
+
+            // This condition can happen if we matched a value that overflows the output type of
+            // its `iconst` when viewed as a signed value (i.e. iconst.i8 200).
+            // When that happens we need to lower as a negative value, which we cannot do here.
+            if !(narrow_mode.is_signed() && is_negative) {
                 return ResultRSEImm12::Imm12(i);
             }
         }
+    }
 
     ResultRSEImm12::from_rse(put_input_in_rse(ctx, input, narrow_mode))
 }

cranelift/filetests/filetests/runtests/icmp.clif

@@ -0,0 +1,17 @@
+test run
+target aarch64
+target s390x
+target x86_64 machinst
+
+; This test is also a regression test for aarch64.
+; We were not correctly handling the fact that the rhs constant value
+; overflows its type when viewed as a signed value, and thus encoding the wrong
+; value into the resulting instruction.
+function %overflow_rhs_const(i8) -> b1 {
+block0(v0: i8):
+    v1 = iconst.i8 192
+    v2 = icmp sge v0, v1
+    return v2
+}
+; run: %test(49) == true
+; run: %test(-65) == false