arm32: fix a bunch of bugs

Signed-off-by: Obei Sideg <obei.sideg@gmail.com>

Author: obeis

cranelift/codegen/meta/src/isa/mod.rs

@@ -34,7 +34,11 @@ impl Isa {
     pub fn from_arch(arch: &str) -> Option<Self> {
         match arch {
             "aarch64" => Some(Isa::Arm64),
-            x if x.starts_with("arm") || x.starts_with("thumb") => Some(Isa::Arm32),
+            // 32-bit ARM / Thumb, but not the 64-bit `arm64*` spellings
+            // (e.g. `arm64ec`), which are AArch64.
+            x if (x.starts_with("arm") || x.starts_with("thumb")) && !x.starts_with("arm64") => {
+                Some(Isa::Arm32)
+            }
             "s390x" => Some(Isa::S390x),
             x if ["x86_64", "i386", "i586", "i686"].contains(&x) => Some(Isa::X86),
             "riscv64" | "riscv64gc" | "riscv64imac" => Some(Isa::Riscv64),

cranelift/codegen/src/isa/arm32/abi.rs

@@ -231,18 +231,36 @@ impl ABIMachineSpec for Arm32MachineDeps {
     }
 
     fn gen_load_base_offset(into_reg: Writable<Reg>, base: Reg, offset: i32, _ty: Type) -> Inst {
-        Inst::Load {
-            rt: into_reg,
-            mem: AMode::RegOffset { rn: base, offset },
-            kind: LoadKind::Word,
+        let mem = AMode::RegOffset { rn: base, offset };
+        if into_reg.to_reg().class() == RegClass::Float {
+            Inst::FpuLoad {
+                size: FpuSize::F64,
+                rd: into_reg,
+                mem,
+            }
+        } else {
+            Inst::Load {
+                rt: into_reg,
+                mem,
+                kind: LoadKind::Word,
+            }
         }
     }
 
     fn gen_store_base_offset(base: Reg, offset: i32, from_reg: Reg, _ty: Type) -> Inst {
-        Inst::Store {
-            rt: from_reg,
-            mem: AMode::RegOffset { rn: base, offset },
-            kind: StoreKind::Word,
+        let mem = AMode::RegOffset { rn: base, offset };
+        if from_reg.class() == RegClass::Float {
+            Inst::FpuStore {
+                size: FpuSize::F64,
+                rt: from_reg,
+                mem,
+            }
+        } else {
+            Inst::Store {
+                rt: from_reg,
+                mem,
+                kind: StoreKind::Word,
+            }
         }
     }
 
@@ -322,27 +340,32 @@ impl ABIMachineSpec for Arm32MachineDeps {
             + frame_layout.outgoing_args_size;
         if stack_size > 0 {
             insts.extend(Self::gen_sp_reg_adjust(-(stack_size as i32)));
-            let mut cur_offset = 0i32;
+            // Callee-saved registers are saved at the *top* of the frame, above
+            // the fixed stack-slot and outgoing-argument regions (which resolve
+            // from sp upward). Placing them from sp=0 would alias those regions
+            // and corrupt saved registers.
+            let mut cur_offset = 0u32;
             for reg in &frame_layout.clobbered_callee_saves {
                 let r = Reg::from(reg.to_reg());
+                let bytes = if r.class() == RegClass::Float { 8 } else { 4 };
+                cur_offset = align_to(cur_offset, bytes);
                 let mem = AMode::SPOffset {
-                    offset: i64::from(cur_offset),
+                    offset: i64::from(stack_size - cur_offset - bytes),
                 };
                 if r.class() == RegClass::Float {
                     insts.push(Inst::FpuStore {
                         size: FpuSize::F64,
                         rt: r,
                         mem,
                     });
-                    cur_offset += 8;
                 } else {
                     insts.push(Inst::Store {
                         rt: r,
                         mem,
                         kind: StoreKind::Word,
                     });
-                    cur_offset += 4;
                 }
+                cur_offset += bytes;
             }
         }
         insts
@@ -357,26 +380,29 @@ impl ABIMachineSpec for Arm32MachineDeps {
         let stack_size = frame_layout.clobber_size
             + frame_layout.fixed_frame_storage_size
             + frame_layout.outgoing_args_size;
-        let mut cur_offset = 0i32;
+        // Mirror the top-of-frame placement used by `gen_clobber_save`.
+        let mut cur_offset = 0u32;
         for reg in &frame_layout.clobbered_callee_saves {
+            let is_float = reg.to_reg().class() == RegClass::Float;
+            let bytes = if is_float { 8 } else { 4 };
+            cur_offset = align_to(cur_offset, bytes);
             let mem = AMode::SPOffset {
-                offset: i64::from(cur_offset),
+                offset: i64::from(stack_size - cur_offset - bytes),
             };
-            if reg.to_reg().class() == RegClass::Float {
+            if is_float {
                 insts.push(Inst::FpuLoad {
                     size: FpuSize::F64,
                     rd: reg.map(Reg::from),
                     mem,
                 });
-                cur_offset += 8;
             } else {
                 insts.push(Inst::Load {
                     rt: reg.map(Reg::from),
                     mem,
                     kind: LoadKind::Word,
                 });
-                cur_offset += 4;
             }
+            cur_offset += bytes;
         }
         if stack_size > 0 {
             insts.extend(Self::gen_sp_reg_adjust(stack_size as i32));

cranelift/codegen/src/isa/arm32/inst.isle

@@ -304,6 +304,10 @@
     ;; A permanently-undefined instruction used to encode a trap.
     (Udf (code TrapCode))
 
+    ;; Conditionally trap on the current flags: emitted as
+    ;; `b<!cond> 1f; udf <code>; 1:`.
+    (TrapIf (cond Cond) (code TrapCode))
+
     ;; Load/store multiple, increment-after (`ldmia`/`stmia rn{!}, {list}`).
     (LdmStm (load bool) (rn Reg) (writeback bool) (reg_list u32))
 
@@ -534,17 +538,12 @@
 (decl pure use_idiv () bool)
 (extern constructor use_idiv use_idiv)
 
-(decl sdiv_reg (Reg Reg) Reg)
-(rule (sdiv_reg rn rm)
-  (let ((rd WritableReg (temp_writable_reg $I32))
-        (_ Unit (emit (MInst.SDiv rd rn rm))))
-    rd))
-
-(decl udiv_reg (Reg Reg) Reg)
-(rule (udiv_reg rn rm)
-  (let ((rd WritableReg (temp_writable_reg $I32))
-        (_ Unit (emit (MInst.UDiv rd rn rm))))
-    rd))
+;; Signed/unsigned divide with the Cranelift-mandated trap checks (divide by
+;; zero, and `INT_MIN / -1` overflow for `sdiv`) emitted before the divide.
+(decl gen_sdiv (Reg Reg) Reg)
+(extern constructor gen_sdiv gen_sdiv)
+(decl gen_udiv (Reg Reg) Reg)
+(extern constructor gen_udiv gen_udiv)
 
 ;;;; Bit operations and extends ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
@@ -699,6 +698,19 @@
 (decl cond_from_floatcc (FloatCC) Cond)
 (extern constructor cond_from_floatcc cond_from_floatcc)
 
+;; The two float conditions that a single ARM condition can't express, built as
+;; a 0/1 boolean from the flags left by a `vcmp`. `one` (ordered and not-equal)
+;; keeps the not-equal result only when ordered (`vc`); `ueq` (unordered or
+;; equal) forces the equal result to 1 when unordered (`vs`).
+(decl gen_fcmp_one (FpuSize Reg Reg) Reg)
+(rule (gen_fcmp_one size a b)
+  (let ((_ Unit (emit_side_effect (vcmp size a b))))
+    (csel (Cond.Vc) (csetv (Cond.Ne)) (gen_constant 0))))
+(decl gen_fcmp_ueq (FpuSize Reg Reg) Reg)
+(rule (gen_fcmp_ueq size a b)
+  (let ((_ Unit (emit_side_effect (vcmp size a b))))
+    (csel (Cond.Vs) (gen_constant 1) (csetv (Cond.Eq)))))
+
 ;; `fcopysign` via GPR bit manipulation (clear the sign of `a`, OR in `b`'s).
 (decl gen_copysign_f32 (Reg Reg) Reg)
 (extern constructor gen_copysign_f32 gen_copysign_f32)
@@ -943,6 +955,12 @@
 (decl cond_from_intcc (IntCC) Cond)
 (extern constructor cond_from_intcc cond_from_intcc)
 
+;; Emit a `cmp` of two (possibly narrow) integer values as a side effect,
+;; widening i8/i16 operands to 32 bits per the comparison's signedness, and
+;; return the ARM condition that then tests the result.
+(decl emit_icmp_cmp (IntCC Value Value) Cond)
+(extern constructor emit_icmp_cmp emit_icmp_cmp)
+
 ;; Emit the compare sequence for a 64-bit `icmp` and return the ARM condition
 ;; that then tests it (the compare itself is emitted as a side effect).
 (decl lower_icmp_i64 (IntCC ValueRegs ValueRegs) Cond)

cranelift/codegen/src/isa/arm32/inst/emit.rs

@@ -748,6 +748,15 @@ impl MachInstEmit for Inst {
                 sink.add_trap(*code);
                 put_u32(sink, enc_udf());
             }
+            Inst::TrapIf { cond, code } => {
+                // Branch over the trap when the condition does not hold.
+                let skip = sink.get_label();
+                sink.use_label_at_offset(sink.cur_offset(), skip, LabelUse::Branch26);
+                put_u32(sink, enc_bcond(cond.invert(), 0));
+                sink.add_trap(*code);
+                put_u32(sink, enc_udf());
+                sink.bind_label(skip, state.ctrl_plane_mut());
+            }
             Inst::LdmStm {
                 load,
                 rn,

cranelift/codegen/src/isa/arm32/inst/mod.rs

@@ -153,7 +153,7 @@ fn arm32_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) {
             use_if_virtual(collector, rm);
             def_if_virtual(collector, rd);
         }
-        Inst::Udf { .. } | Inst::Barrier { .. } => {}
+        Inst::Udf { .. } | Inst::TrapIf { .. } | Inst::Barrier { .. } => {}
 
         Inst::AtomicRmw {
             rd,
@@ -551,9 +551,12 @@ impl MachInst for Inst {
     }
 
     fn worst_case_size() -> CodeOffset {
-        // The largest instruction is `MovImm`, which expands to a `movw` plus a
-        // `movt`: 8 bytes.
-        8
+        // Must be an upper bound on the number of bytes any single (non
+        // jump-table) instruction emits, since the buffer uses it as the
+        // per-instruction island lookahead. The largest such sequence is the
+        // 64-bit atomic min/max RMW loop, which expands to 10 words (40 bytes).
+        // Inline jump tables manage their own islands and are exempt.
+        44
     }
 
     fn worst_case_island_growth() -> CodeOffset {
@@ -771,6 +774,9 @@ impl Inst {
                 alloc::format!("rrx {}, {}", r(rd.to_reg()), r(*rm))
             }
             Inst::Udf { code } => alloc::format!("udf ; {code}"),
+            Inst::TrapIf { cond, code } => {
+                alloc::format!("b{} 1f ; udf ; 1: ; {code}", cond.invert().name())
+            }
             Inst::LdmStm {
                 load,
                 rn,

cranelift/codegen/src/isa/arm32/lower.isle

@@ -208,6 +208,17 @@
   (let ((_ Unit (emit_side_effect (vcmp (FpuSize.F64) (put_in_reg a) (put_in_reg b)))))
     (csetv (cond_from_floatcc cc))))
 
+;; `one` (ordered and not-equal) and `ueq` (unordered or equal) can't be
+;; expressed as a single ARM condition, so they get a two-condition sequence.
+(rule 1 (lower (fcmp _ (FloatCC.OrderedNotEqual) a @ (value_type $F32) b))
+  (gen_fcmp_one (FpuSize.F32) (put_in_reg a) (put_in_reg b)))
+(rule 1 (lower (fcmp _ (FloatCC.OrderedNotEqual) a @ (value_type $F64) b))
+  (gen_fcmp_one (FpuSize.F64) (put_in_reg a) (put_in_reg b)))
+(rule 1 (lower (fcmp _ (FloatCC.UnorderedOrEqual) a @ (value_type $F32) b))
+  (gen_fcmp_ueq (FpuSize.F32) (put_in_reg a) (put_in_reg b)))
+(rule 1 (lower (fcmp _ (FloatCC.UnorderedOrEqual) a @ (value_type $F64) b))
+  (gen_fcmp_ueq (FpuSize.F64) (put_in_reg a) (put_in_reg b)))
+
 ;; f32 <-> f64.
 (rule (lower (fpromote $F64 x))
   (vcvt_ff true (put_in_reg x)))
@@ -266,14 +277,15 @@
   (mov_to_fpu64 (put_in_regs x)))
 
 ;;;; Divides (only when hardware `sdiv`/`udiv` is available) ;;;;;;;;;;;;;;;;;;;
-;; NOTE: trap-on-zero / INT_MIN overflow checks are not yet emitted.
+;; `gen_sdiv`/`gen_udiv` emit the trap-on-zero check (and, for `sdiv`, the
+;; `INT_MIN / -1` overflow check) before the divide.
 
 (rule (lower (sdiv $I32 x y))
   (if-let true (use_idiv))
-  (sdiv_reg (put_in_reg x) (put_in_reg y)))
+  (gen_sdiv (put_in_reg x) (put_in_reg y)))
 (rule (lower (udiv $I32 x y))
   (if-let true (use_idiv))
-  (udiv_reg (put_in_reg x) (put_in_reg y)))
+  (gen_udiv (put_in_reg x) (put_in_reg y)))
 
 ;;;; Counting / byte-swap / bit-reverse ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
@@ -304,10 +316,11 @@
 
 ;;;; `icmp` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-;; 32-bit comparison producing a 0/1 boolean.
+;; 32-bit comparison producing a 0/1 boolean. `emit_icmp_cmp` widens narrow
+;; (i8/i16) operands to 32 bits per the comparison's signedness before the
+;; `cmp`, since Cranelift leaves the high bits of narrow values undefined.
 (rule (lower (icmp _ cc a @ (value_type (fits_in_32 _)) b))
-  (let ((_ Unit (emit_side_effect (cmp_rr (put_in_reg a) (put_in_reg b)))))
-    (csetv (cond_from_intcc cc))))
+  (csetv (emit_icmp_cmp cc a b)))
 
 ;; 64-bit comparison producing a 0/1 boolean.
 (rule 4 (lower (icmp _ cc a @ (value_type $I64) b))
@@ -317,14 +330,23 @@
 
 ;; Fuse an `icmp` condition into the compare that precedes the select.
 (rule 1 (lower (select (fits_in_32 _) (icmp _ cc x @ (value_type (fits_in_32 _)) y) a b))
-  (let ((_ Unit (emit_side_effect (cmp_rr (put_in_reg x) (put_in_reg y)))))
-    (csel (cond_from_intcc cc) (put_in_reg a) (put_in_reg b))))
+  (let ((cond Cond (emit_icmp_cmp cc x y)))
+    (csel cond (put_in_reg a) (put_in_reg b))))
 
 ;; Generic select: branch on whether the condition value is non-zero.
 (rule (lower (select (fits_in_32 _) c a b))
   (let ((_ Unit (emit_side_effect (cmp_imm (put_in_reg c) 0))))
     (csel (Cond.Ne) (put_in_reg a) (put_in_reg b))))
 
+;; 64-bit select: pick each half of the pair. The operands are materialized
+;; before the compare so nothing clobbers the flags in between.
+(rule 4 (lower (select $I64 c a b))
+  (let ((av ValueRegs (put_in_regs a))
+        (bv ValueRegs (put_in_regs b))
+        (_ Unit (emit_side_effect (cmp_imm (put_in_reg c) 0))))
+    (value_regs (csel (Cond.Ne) (vr_lo av) (vr_lo bv))
+                (csel (Cond.Ne) (vr_hi av) (vr_hi bv)))))
+
 ;;;; Shifts: `ishl` / `ushr` / `sshr` / `rotr` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (rule 1 (lower (ishl $I32 x (iconst _ (u64_from_imm64 n))))
@@ -349,6 +371,12 @@
 
 ;;;; Loads ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
+;; A plain `load` reads exactly `sizeof(ty)` bytes; narrow types zero-extend
+;; into the 32-bit result register.
+(rule 1 (lower (load $I8 flags addr offset))
+  (arm_load (amode addr offset) (LoadKind.UByte)))
+(rule 1 (lower (load $I16 flags addr offset))
+  (arm_load (amode addr offset) (LoadKind.UHalf)))
 (rule (lower (load (fits_in_32 _) flags addr offset))
   (arm_load (amode addr offset) (LoadKind.Word)))
 
@@ -363,6 +391,11 @@
 
 ;;;; Stores ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
+;; A plain `store` writes exactly `sizeof(ty)` bytes.
+(rule 1 (lower (store flags val @ (value_type $I8) addr offset))
+  (arm_store (put_in_reg val) (amode addr offset) (StoreKind.Byte)))
+(rule 1 (lower (store flags val @ (value_type $I16) addr offset))
+  (arm_store (put_in_reg val) (amode addr offset) (StoreKind.Half)))
 (rule (lower (store flags val @ (value_type (fits_in_32 _)) addr offset))
   (arm_store (put_in_reg val) (amode addr offset) (StoreKind.Word)))
 
@@ -431,16 +464,33 @@
 
 ;; Fuse a 32-bit `icmp` feeding a `brif` into a compare plus conditional branch.
 (rule 1 (lower_branch (brif (icmp _ cc a @ (value_type (fits_in_32 _)) b) _ _) (two_targets taken not_taken))
-  (emit_side_effect
-    (side_effect_concat
-      (cmp_rr (put_in_reg a) (put_in_reg b))
-      (cond_br (cond_from_intcc cc) taken not_taken))))
+  (emit_side_effect (cond_br (emit_icmp_cmp cc a b) taken not_taken)))
 
 ;; Fuse a 64-bit `icmp` feeding a `brif`.
 (rule 2 (lower_branch (brif (icmp _ cc a @ (value_type $I64) b) _ _) (two_targets taken not_taken))
   (emit_side_effect
     (cond_br (lower_icmp_i64 cc (put_in_regs a) (put_in_regs b)) taken not_taken)))
 
+;; `one`/`ueq` feeding a `brif`: materialize the two-condition boolean, then
+;; branch on whether it is non-zero (priority 2 beats the generic fused rule,
+;; whose single-condition mapping can't represent these).
+(rule 2 (lower_branch (brif (fcmp _ (FloatCC.OrderedNotEqual) a @ (value_type $F32) b) _ _) (two_targets taken not_taken))
+  (emit_side_effect (side_effect_concat
+    (cmp_imm (gen_fcmp_one (FpuSize.F32) (put_in_reg a) (put_in_reg b)) 0)
+    (cond_br (Cond.Ne) taken not_taken))))
+(rule 2 (lower_branch (brif (fcmp _ (FloatCC.OrderedNotEqual) a @ (value_type $F64) b) _ _) (two_targets taken not_taken))
+  (emit_side_effect (side_effect_concat
+    (cmp_imm (gen_fcmp_one (FpuSize.F64) (put_in_reg a) (put_in_reg b)) 0)
+    (cond_br (Cond.Ne) taken not_taken))))
+(rule 2 (lower_branch (brif (fcmp _ (FloatCC.UnorderedOrEqual) a @ (value_type $F32) b) _ _) (two_targets taken not_taken))
+  (emit_side_effect (side_effect_concat
+    (cmp_imm (gen_fcmp_ueq (FpuSize.F32) (put_in_reg a) (put_in_reg b)) 0)
+    (cond_br (Cond.Ne) taken not_taken))))
+(rule 2 (lower_branch (brif (fcmp _ (FloatCC.UnorderedOrEqual) a @ (value_type $F64) b) _ _) (two_targets taken not_taken))
+  (emit_side_effect (side_effect_concat
+    (cmp_imm (gen_fcmp_ueq (FpuSize.F64) (put_in_reg a) (put_in_reg b)) 0)
+    (cond_br (Cond.Ne) taken not_taken))))
+
 ;; Fuse an `fcmp` feeding a `brif`.
 (rule 1 (lower_branch (brif (fcmp _ cc a @ (value_type $F32) b) _ _) (two_targets taken not_taken))
   (emit_side_effect