From b1e2c4af02a64525e922263cf1a4ac077143068e Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Fri, 21 Aug 2026 17:41:12 +1000 Subject: [PATCH 1/8] Add `BitfieldMove` to AArch64 MInst enum, and a lowering that combines eligible `ishl`/`sshr` pairs into one `sbfm` instruction. --- cranelift/codegen/src/isa/aarch64/inst.isle | 38 +++++++++++++++++++ .../codegen/src/isa/aarch64/inst/emit.rs | 35 +++++++++++++---- .../codegen/src/isa/aarch64/inst/imms.rs | 31 ++++++++++++--- cranelift/codegen/src/isa/aarch64/inst/mod.rs | 38 +++++++++++++++++-- cranelift/codegen/src/isa/aarch64/lower.isle | 6 +++ .../codegen/src/isa/aarch64/lower/isle.rs | 6 ++- .../filetests/isa/aarch64/shift-op.clif | 1 - .../filetests/isa/aarch64/shift-rotate.clif | 18 +++++++++ 8 files changed, 155 insertions(+), 18 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 6bae128cecf8..216351fadbf4 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -209,6 +209,16 @@ (from_bits u8) (to_bits u8)) + ;; A bitfield move instruction, which encompasses + ;; the BFM, UBFM and SBFM instructions. + (BitfieldMove + (size OperandSize) + (bfm_op BfmOp) + (rd WritableReg) + (rn Reg) + (immr UImm6) + (imms UImm6)) + ;; A conditional-select operation. (CSel (rd WritableReg) @@ -1263,6 +1273,14 @@ (MovN) )) +;; A bitfield move operation. +(type BfmOp + (enum + (Bfm) + (UBfm) + (SBfm) +)) + (model UImm5 (type (bv 5))) (type UImm5 (primitive UImm5)) @@ -1282,6 +1300,9 @@ (model ImmShift (type (bv 6))) (type ImmShift (primitive ImmShift)) +(model UImm6 (type (bv 6))) +(type UImm6 (primitive UImm6)) + (model ShiftOpAndAmt (type (struct @@ -2188,6 +2209,13 @@ (decl imm_shift_from_u8 (u8) ImmShift) (extern constructor imm_shift_from_u8 imm_shift_from_u8) +(spec (uimm6_from_u8 n) + (provide (= result (extract 5 0 n))) + (require (bvult n #x40)) +) +(decl uimm6_from_u8 (u8) UImm6) +(extern constructor uimm6_from_u8 uimm6_from_u8) + (spec (imm12_from_u64 imm12) (provide (= result @@ -2954,6 +2982,16 @@ (_ Unit (emit (MInst.Extend dst rn signed from_bits to_bits)))) dst)) +;; Helper for emitting `MInst.BitfieldMove` instructions. +(attr bitfield_move (veri chain)) +(decl bitfield_move (Type BfmOp Reg u8 u8) Reg) +(rule (bitfield_move ty bfm_op rn immr imms) + (let ((dst WritableReg (temp_writable_reg ty)) + (immr UImm6 (uimm6_from_u8 immr)) + (imms UImm6 (uimm6_from_u8 imms)) + (_ Unit (emit (MInst.BitfieldMove (operand_size ty) bfm_op dst rn immr imms)))) + dst)) + ;; Helper for emitting `MInst.FpuExtend` instructions. (attr fpu_extend (veri chain)) (decl fpu_extend (Reg ScalarSize) Reg) diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs index a603e8d6e0fa..fb44b72f9b22 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit.rs @@ -417,7 +417,14 @@ fn enc_ccmp_imm(size: OperandSize, rn: Reg, imm: UImm5, nzcv: NZCV, cond: Cond) | nzcv.bits() } -fn enc_bfm(opc: u8, size: OperandSize, rd: Writable, rn: Reg, immr: u8, imms: u8) -> u32 { +fn enc_bfm( + bfm_op: BfmOp, + size: OperandSize, + rd: Writable, + rn: Reg, + immr: u8, + imms: u8, +) -> u32 { match size { OperandSize::Size64 => { debug_assert!(immr <= 63); @@ -428,11 +435,15 @@ fn enc_bfm(opc: u8, size: OperandSize, rd: Writable, rn: Reg, immr: u8, imm debug_assert!(imms <= 31); } } - debug_assert_eq!(opc & 0b11, opc); + let opc = match bfm_op { + BfmOp::Bfm => 0b01, + BfmOp::UBfm => 0b10, + BfmOp::SBfm => 0b00, + }; let n_bit = size.sf_bit(); 0b0_00_100110_0_000000_000000_00000_00000 | size.sf_bit() << 31 - | u32::from(opc) << 29 + | opc << 29 | n_bit << 22 | u32::from(immr) << 16 | u32::from(imms) << 10 @@ -2920,12 +2931,22 @@ impl MachInstEmit for Inst { from_bits, to_bits, } => { - let (opc, size) = if signed { - (0b00, OperandSize::from_bits(to_bits)) + let (bfm_op, size) = if signed { + (BfmOp::SBfm, OperandSize::from_bits(to_bits)) } else { - (0b10, OperandSize::Size32) + (BfmOp::UBfm, OperandSize::Size32) }; - sink.put4(enc_bfm(opc, size, rd, rn, 0, from_bits - 1)); + sink.put4(enc_bfm(bfm_op, size, rd, rn, 0, from_bits - 1)); + } + &Inst::BitfieldMove { + size, + bfm_op, + rd, + rn, + immr, + imms, + } => { + sink.put4(enc_bfm(bfm_op, size, rd, rn, immr.value(), imms.value())); } &Inst::Jump { ref dest } => { let off = sink.cur_offset(); diff --git a/cranelift/codegen/src/isa/aarch64/inst/imms.rs b/cranelift/codegen/src/isa/aarch64/inst/imms.rs index 72e26095e28d..b5c72cb4a22a 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/imms.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/imms.rs @@ -547,11 +547,26 @@ pub struct ImmShift { impl ImmShift { /// Create an ImmShift from raw bits, if possible. pub fn maybe_from_u64(val: u64) -> Option { - if val < 64 { - Some(ImmShift { imm: val as u8 }) - } else { - None - } + (val < 64).then_some(ImmShift { imm: val as u8 }) + } + + /// Get the immediate value. + pub fn value(&self) -> u8 { + self.imm + } +} + +/// A 6-bit immediate used by the `immr` and `imms` fields of bitfield move instructions. +#[derive(Copy, Clone, Debug)] +pub struct UImm6 { + /// 6-bit immediate. + pub imm: u8, +} + +impl UImm6 { + /// Create a UImm6 from raw bits, if possible. + pub fn maybe_from_u8(val: u8) -> Option { + (val < 64).then_some(UImm6 { imm: val }) } /// Get the immediate value. @@ -915,6 +930,12 @@ impl PrettyPrint for ImmShift { } } +impl PrettyPrint for UImm6 { + fn pretty_print(&self, _: u8) -> String { + format!("#{}", self.imm) + } +} + impl PrettyPrint for MoveWideConst { fn pretty_print(&self, _: u8) -> String { if self.shift == 0 { diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index 4887b75afb5b..1d9e896e3348 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -34,10 +34,10 @@ mod emit_tests; // Instructions (top level): definition pub use crate::isa::aarch64::lower::isle::generated_code::{ - ALUOp, ALUOp3, AMode, APIKey, AtomicRMWLoopOp, AtomicRMWOp, BitOp, BranchTargetType, FPUOp1, - FPUOp2, FPUOp3, FpuRoundMode, FpuToIntOp, IntToFpuOp, MInst as Inst, MoveWideOp, VecALUModOp, - VecALUOp, VecExtendOp, VecLanesOp, VecMisc2, VecPairOp, VecRRLongOp, VecRRNarrowOp, - VecRRPairLongOp, VecRRRLongModOp, VecRRRLongOp, VecShiftImmModOp, VecShiftImmOp, + ALUOp, ALUOp3, AMode, APIKey, AtomicRMWLoopOp, AtomicRMWOp, BfmOp, BitOp, BranchTargetType, + FPUOp1, FPUOp2, FPUOp3, FpuRoundMode, FpuToIntOp, IntToFpuOp, MInst as Inst, MoveWideOp, + VecALUModOp, VecALUOp, VecExtendOp, VecLanesOp, VecMisc2, VecPairOp, VecRRLongOp, + VecRRNarrowOp, VecRRPairLongOp, VecRRRLongModOp, VecRRRLongOp, VecShiftImmModOp, VecShiftImmOp, }; /// A floating-point unit (FPU) operation with two args, a register and an immediate. @@ -60,6 +60,17 @@ pub enum FPUOpRIMod { Sli64(FPULeftShiftImm), } +impl BfmOp { + /// Get the assembly mnemonic for this opcode. + pub fn op_str(&self) -> &'static str { + match self { + BfmOp::Bfm => "bfm", + BfmOp::UBfm => "ubfm", + BfmOp::SBfm => "sbfm", + } + } +} + impl BitOp { /// Get the assembly mnemonic for this opcode. pub fn op_str(&self) -> &'static str { @@ -791,6 +802,10 @@ fn aarch64_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) { collector.reg_def(rd); collector.reg_use(rn); } + Inst::BitfieldMove { rd, rn, .. } => { + collector.reg_def(rd); + collector.reg_use(rn); + } Inst::Args { args } => { for ArgPair { vreg, preg } in args { collector.reg_fixed_def(vreg, *preg); @@ -2594,6 +2609,21 @@ impl Inst { format!("{op} {rd}, {rn}") } } + &Inst::BitfieldMove { + size, + bfm_op, + rd, + rn, + immr, + imms, + } => { + let op = bfm_op.op_str(); + let rd = pretty_print_ireg(rd.to_reg(), size); + let rn = pretty_print_ireg(rn, size); + let immr = immr.pretty_print(0); + let imms = imms.pretty_print(0); + format!("{op} {rd}, {rn}, {immr}, {imms}") + } &Inst::Call { ref info } => { let try_call = info .try_call_info diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index f3c37ff60348..bf512cfafe63 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1699,6 +1699,12 @@ (rule sshr_64 (lower (sshr $I64 x y)) (do_shift (ALUOp.Asr) $I64 (put_in_reg_sext64 x) y)) +;; Specialized lowerings to generate the `sbfm` instruction. +(rule sbfm 1 (lower + (sshr $I64 (ishl $I64 x (u8_from_iconst a)) (u8_from_iconst b))) + (if-let true (u8_lt_eq a b)) + (bitfield_move $I64 (BfmOp.SBfm) x (u8_sub b a) (u8_sub 63 a))) + ;; Shift for i128. (rule (lower (sshr $I128 x y)) (lower_sshr128 x (value_regs_get y 0))) diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs index 4ce099d73b20..e046db99582a 100644 --- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs +++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs @@ -9,7 +9,7 @@ use super::{ ASIMDFPModImm, ASIMDMovModImm, BranchTarget, CallInfo, Cond, CondBrKind, ExtendOp, FPUOpRI, FPUOpRIMod, FloatCC, Imm12, ImmLogic, ImmShift, Inst as MInst, IntCC, MachLabel, MemLabel, MoveWideConst, MoveWideOp, NZCV, Opcode, OperandSize, Reg, SImm9, ScalarSize, ShiftOpAndAmt, - UImm5, UImm12Scaled, VecMisc2, VectorSize, fp_reg, lower_condcode, stack_reg, + UImm5, UImm6, UImm12Scaled, VecMisc2, VectorSize, fp_reg, lower_condcode, stack_reg, writable_link_reg, writable_zero_reg, zero_reg, }; use crate::ir::{ArgumentExtension, condcodes}; @@ -241,6 +241,10 @@ impl Context for IsleContext<'_, '_, MInst, AArch64Backend> { ImmShift::maybe_from_u64(n.into()).unwrap() } + fn uimm6_from_u8(&mut self, n: u8) -> UImm6 { + UImm6::maybe_from_u8(n.into()).unwrap() + } + fn lshr_from_u64(&mut self, ty: Type, n: u64) -> Option { let shiftimm = ShiftOpShiftImm::maybe_from_shift(n)?; if let Ok(bits) = u8::try_from(ty_bits(ty)) { diff --git a/cranelift/filetests/filetests/isa/aarch64/shift-op.clif b/cranelift/filetests/filetests/isa/aarch64/shift-op.clif index d0c0c972b017..56e78b7c080c 100644 --- a/cranelift/filetests/filetests/isa/aarch64/shift-op.clif +++ b/cranelift/filetests/filetests/isa/aarch64/shift-op.clif @@ -36,4 +36,3 @@ block0(v0: i32): ; block0: ; offset 0x0 ; lsl w0, w0, #0x15 ; ret - diff --git a/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif b/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif index 42d066572af0..c9a9367cc822 100644 --- a/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif +++ b/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif @@ -666,3 +666,21 @@ block0(v0: i64): ; lsl x0, x0, #0x11 ; ret +function %f28(i64) -> i64 { +block0(v0: i64): + v1 = iconst.i32 32 + v2 = ishl.i64 v0, v1 + v3 = iconst.i32 52 + v4 = sshr.i64 v2, v3 + return v4 +} + +; VCode: +; block0: +; sbfm x0, x0, #20, #31 +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; sbfx x0, x0, #0x14, #0xc +; ret From 377b1576c31aa48552770c4307acc8aa1fc2b7c7 Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Fri, 21 Aug 2026 18:51:15 +1000 Subject: [PATCH 2/8] Generalised the `sbfm` lowering rule to cover more cases; added helper methods `sbfm_immr`/`sbfm_imms`; added additional test cases --- cranelift/codegen/src/isa/aarch64/inst.isle | 11 +-- cranelift/codegen/src/isa/aarch64/lower.isle | 14 +++- .../codegen/src/isa/aarch64/lower/isle.rs | 17 +++- .../filetests/isa/aarch64/shift-rotate.clif | 79 +++++++++++++++++++ 4 files changed, 105 insertions(+), 16 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 216351fadbf4..421d8397730d 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -2209,13 +2209,6 @@ (decl imm_shift_from_u8 (u8) ImmShift) (extern constructor imm_shift_from_u8 imm_shift_from_u8) -(spec (uimm6_from_u8 n) - (provide (= result (extract 5 0 n))) - (require (bvult n #x40)) -) -(decl uimm6_from_u8 (u8) UImm6) -(extern constructor uimm6_from_u8 uimm6_from_u8) - (spec (imm12_from_u64 imm12) (provide (= result @@ -2984,11 +2977,9 @@ ;; Helper for emitting `MInst.BitfieldMove` instructions. (attr bitfield_move (veri chain)) -(decl bitfield_move (Type BfmOp Reg u8 u8) Reg) +(decl bitfield_move (Type BfmOp Reg UImm6 UImm6) Reg) (rule (bitfield_move ty bfm_op rn immr imms) (let ((dst WritableReg (temp_writable_reg ty)) - (immr UImm6 (uimm6_from_u8 immr)) - (imms UImm6 (uimm6_from_u8 imms)) (_ Unit (emit (MInst.BitfieldMove (operand_size ty) bfm_op dst rn immr imms)))) dst)) diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index bf512cfafe63..a230706ccb52 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1699,11 +1699,17 @@ (rule sshr_64 (lower (sshr $I64 x y)) (do_shift (ALUOp.Asr) $I64 (put_in_reg_sext64 x) y)) -;; Specialized lowerings to generate the `sbfm` instruction. +;; Specialized lowerings to generate the `sbfm` instruction from +;; an appropriate pair of `ishl` and `ushr`/`sshr` instructions. (rule sbfm 1 (lower - (sshr $I64 (ishl $I64 x (u8_from_iconst a)) (u8_from_iconst b))) - (if-let true (u8_lt_eq a b)) - (bitfield_move $I64 (BfmOp.SBfm) x (u8_sub b a) (u8_sub 63 a))) + (sshr (ty_32_or_64 ty) (ishl _ x (u64_from_iconst a)) (u64_from_iconst b))) + (bitfield_move ty (BfmOp.SBfm) x (sbfm_immr ty a b) (sbfm_imms ty a b))) + +;; Helper methods for constructing the correct `immr` and `imms` immediates. +(decl sbfm_immr (Type u64 u64) UImm6) +(extern constructor sbfm_immr sbfm_immr) +(decl sbfm_imms (Type u64 u64) UImm6) +(extern constructor sbfm_imms sbfm_imms) ;; Shift for i128. (rule (lower (sshr $I128 x y)) diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs index e046db99582a..c53a8550b1b4 100644 --- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs +++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs @@ -241,8 +241,21 @@ impl Context for IsleContext<'_, '_, MInst, AArch64Backend> { ImmShift::maybe_from_u64(n.into()).unwrap() } - fn uimm6_from_u8(&mut self, n: u8) -> UImm6 { - UImm6::maybe_from_u8(n.into()).unwrap() + /// Compute the `immr` value for an `sbfm` instruction, + /// derived by fusing an `ishl` by amount `a`, with an `sshr` by amount `b`. + fn sbfm_immr(&mut self, ty: Type, a: u64, b: u64) -> UImm6 { + let w = ty.lane_bits() as u8; + let a = (a as u8) & (w - 1); + let b = (b as u8) & (w - 1); + UImm6::maybe_from_u8(if a <= b { b - a } else { w - (a - b) }).unwrap() + } + + /// Compute the `imms` value for an `sbfm` instruction, + /// derived by fusing an `ishl` by amount `a`, with an `sshr` by amount `b`. + fn sbfm_imms(&mut self, ty: Type, a: u64, _b: u64) -> UImm6 { + let w = ty.lane_bits() as u8; + let a = (a as u8) & (w - 1); + UImm6::maybe_from_u8(w - 1 - (a & (w - 1))).unwrap() } fn lshr_from_u64(&mut self, ty: Type, n: u64) -> Option { diff --git a/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif b/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif index c9a9367cc822..8df3c6763a03 100644 --- a/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif +++ b/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif @@ -684,3 +684,82 @@ block0(v0: i64): ; block0: ; offset 0x0 ; sbfx x0, x0, #0x14, #0xc ; ret + +function %f29(i32) -> i32 { +block0(v0: i32): + v1 = iconst.i32 16 + v2 = ishl.i32 v0, v1 + v3 = iconst.i32 26 + v4 = sshr.i32 v2, v3 + return v4 +} + +; VCode: +; block0: +; sbfm w0, w0, #10, #15 +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; sbfx w0, w0, #0xa, #6 +; ret + +function %f30(i64) -> i64 { +block0(v0: i64): + v1 = iconst.i32 40 + v2 = ishl.i64 v0, v1 + v3 = iconst.i32 20 + v4 = sshr.i64 v2, v3 + return v4 +} + +; VCode: +; block0: +; sbfm x0, x0, #44, #23 +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; sbfiz x0, x0, #0x14, #0x18 +; ret + +function %f31(i32) -> i32 { +block0(v0: i32): + v1 = iconst.i32 12 + v2 = ishl.i32 v0, v1 + v3 = iconst.i32 8 + v4 = sshr.i32 v2, v3 + return v4 +} + +; VCode: +; block0: +; sbfm w0, w0, #28, #19 +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; sbfiz w0, w0, #4, #0x14 +; ret + +function %f32(i64) -> i64 { +block0(v0: i64): + v1 = iconst.i32 12 + v2 = ishl.i64 v0, v1 + v3 = iconst.i32 20 + v4 = sshr.i64 v2, v3 + v5 = iadd.i64 v2, v4 + return v5 +} + +; VCode: +; block0: +; sbfm x3, x0, #8, #51 +; add x0, x3, x0, LSL 12 +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; sbfx x3, x0, #8, #0x2c +; add x0, x3, x0, lsl #12 +; ret From 7211dff8303ae989b686205c7201406f9f6419c5 Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Fri, 21 Aug 2026 18:53:55 +1000 Subject: [PATCH 3/8] Add `ubfm` rule, the unsigned analogue of `sbfm`, with test cases --- cranelift/codegen/src/isa/aarch64/lower.isle | 3 ++ .../filetests/isa/aarch64/shift-rotate.clif | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index a230706ccb52..72afc065e327 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1704,6 +1704,9 @@ (rule sbfm 1 (lower (sshr (ty_32_or_64 ty) (ishl _ x (u64_from_iconst a)) (u64_from_iconst b))) (bitfield_move ty (BfmOp.SBfm) x (sbfm_immr ty a b) (sbfm_imms ty a b))) +(rule ubfm 1 (lower + (ushr (ty_32_or_64 ty) (ishl _ x (u64_from_iconst a)) (u64_from_iconst b))) + (bitfield_move ty (BfmOp.UBfm) x (sbfm_immr ty a b) (sbfm_imms ty a b))) ;; Helper methods for constructing the correct `immr` and `imms` immediates. (decl sbfm_immr (Type u64 u64) UImm6) diff --git a/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif b/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif index 8df3c6763a03..dcd267713a17 100644 --- a/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif +++ b/cranelift/filetests/filetests/isa/aarch64/shift-rotate.clif @@ -763,3 +763,42 @@ block0(v0: i64): ; sbfx x3, x0, #8, #0x2c ; add x0, x3, x0, lsl #12 ; ret + +function %f33(i64) -> i64 { +block0(v0: i64): + v1 = iconst.i32 48 + v2 = ishl.i64 v0, v1 + v3 = iconst.i32 54 + v4 = ushr.i64 v2, v3 + return v4 +} + +; VCode: +; block0: +; ubfm x0, x0, #6, #15 +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ubfx x0, x0, #6, #0xa +; ret + +function %f34(i64) -> i64 { +block0(v0: i64): + v1 = iconst.i32 48 + v2 = ishl.i64 v0, v1 + v3 = iconst.i32 32 + v4 = ushr.i64 v2, v3 + return v4 +} + +; VCode: +; block0: +; ubfm x0, x0, #48, #15 +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ubfiz x0, x0, #0x10, #0x10 +; ret + From 882b9949da68330819fe4ce17665513467f4cfb6 Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Tue, 25 Aug 2026 10:01:17 +1000 Subject: [PATCH 4/8] PR feedback; remove BFM from `BfmOp`, as it has modify semantics on `rd` --- cranelift/codegen/src/isa/aarch64/inst.isle | 7 ++++--- cranelift/codegen/src/isa/aarch64/inst/emit.rs | 2 +- cranelift/codegen/src/isa/aarch64/inst/mod.rs | 5 ++++- cranelift/codegen/src/isa/aarch64/lower.isle | 4 ++-- cranelift/codegen/src/isa/aarch64/lower/isle.rs | 10 ++++++++-- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 421d8397730d..1fa607948ef2 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -209,8 +209,9 @@ (from_bits u8) (to_bits u8)) - ;; A bitfield move instruction, which encompasses - ;; the BFM, UBFM and SBFM instructions. + ;; A bitfield move instruction, either UBFM or SBFM. + ;; BFM has been intentionally excluded as it leaves some bits + ;; in `rd` unchanged, rather than overwriting all of them. (BitfieldMove (size OperandSize) (bfm_op BfmOp) @@ -1274,9 +1275,9 @@ )) ;; A bitfield move operation. +;; Note that BFM is excluded as it modifies rather than overwrites `rd`. (type BfmOp (enum - (Bfm) (UBfm) (SBfm) )) diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs index fb44b72f9b22..716f8e207233 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit.rs @@ -436,9 +436,9 @@ fn enc_bfm( } } let opc = match bfm_op { - BfmOp::Bfm => 0b01, BfmOp::UBfm => 0b10, BfmOp::SBfm => 0b00, + // Note: BFM (`01`) is intentionally excluded }; let n_bit = size.sf_bit(); 0b0_00_100110_0_000000_000000_00000_00000 diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index 1d9e896e3348..2f91ae06f105 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -64,7 +64,6 @@ impl BfmOp { /// Get the assembly mnemonic for this opcode. pub fn op_str(&self) -> &'static str { match self { - BfmOp::Bfm => "bfm", BfmOp::UBfm => "ubfm", BfmOp::SBfm => "sbfm", } @@ -803,6 +802,10 @@ fn aarch64_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) { collector.reg_use(rn); } Inst::BitfieldMove { rd, rn, .. } => { + // BFM has been excluded from this instruction format + // as it can leave some bits of `rd` unchanged. + // In contrast, the UBFM and SBFM instructions always + // replace all bits in `rd`, making it a true def. collector.reg_def(rd); collector.reg_use(rn); } diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 72afc065e327..1f992c75021a 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1699,8 +1699,8 @@ (rule sshr_64 (lower (sshr $I64 x y)) (do_shift (ALUOp.Asr) $I64 (put_in_reg_sext64 x) y)) -;; Specialized lowerings to generate the `sbfm` instruction from -;; an appropriate pair of `ishl` and `ushr`/`sshr` instructions. +;; Specialized lowerings to generate a single `ubfm`/`sbfm` instruction from +;; an appropriate pair of `ishl` and `ushr`/`sshr` operations. (rule sbfm 1 (lower (sshr (ty_32_or_64 ty) (ishl _ x (u64_from_iconst a)) (u64_from_iconst b))) (bitfield_move ty (BfmOp.SBfm) x (sbfm_immr ty a b) (sbfm_imms ty a b))) diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs index c53a8550b1b4..27354cf8a552 100644 --- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs +++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs @@ -245,17 +245,23 @@ impl Context for IsleContext<'_, '_, MInst, AArch64Backend> { /// derived by fusing an `ishl` by amount `a`, with an `sshr` by amount `b`. fn sbfm_immr(&mut self, ty: Type, a: u64, b: u64) -> UImm6 { let w = ty.lane_bits() as u8; + debug_assert!(w <= 64); + let a = (a as u8) & (w - 1); let b = (b as u8) & (w - 1); - UImm6::maybe_from_u8(if a <= b { b - a } else { w - (a - b) }).unwrap() + let result = if a <= b { b - a } else { w - (a - b) }; + UImm6::maybe_from_u8(result).expect("result is always less than 64") } /// Compute the `imms` value for an `sbfm` instruction, /// derived by fusing an `ishl` by amount `a`, with an `sshr` by amount `b`. fn sbfm_imms(&mut self, ty: Type, a: u64, _b: u64) -> UImm6 { let w = ty.lane_bits() as u8; + debug_assert!(w <= 64); + let a = (a as u8) & (w - 1); - UImm6::maybe_from_u8(w - 1 - (a & (w - 1))).unwrap() + let result = w - 1 - (a & (w - 1)); + UImm6::maybe_from_u8(result).expect("result is always less than 64") } fn lshr_from_u64(&mut self, ty: Type, n: u64) -> Option { From f8b1ef114f2e6ebad67749cd5659864cb8c3a441 Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Tue, 25 Aug 2026 11:38:54 +1000 Subject: [PATCH 5/8] aarch64: add representation for BFM instruction --- cranelift/codegen/src/isa/aarch64/inst.isle | 14 +++++- .../codegen/src/isa/aarch64/inst/emit.rs | 43 ++++++++++++------- cranelift/codegen/src/isa/aarch64/inst/mod.rs | 26 +++++++++-- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 1fa607948ef2..ecc59d2fc27c 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -210,8 +210,7 @@ (to_bits u8)) ;; A bitfield move instruction, either UBFM or SBFM. - ;; BFM has been intentionally excluded as it leaves some bits - ;; in `rd` unchanged, rather than overwriting all of them. + ;; The BFM instruction is instead encoded as BitfieldMoveMod. (BitfieldMove (size OperandSize) (bfm_op BfmOp) @@ -220,6 +219,17 @@ (immr UImm6) (imms UImm6)) + ;; The bitfield move instruction that modifies its `rd`, i.e. BFM. + ;; The input state of `rd` is represented as `ri` (for "input"), + ;; and is constrained with `rd` so they use the same register. + (BitfieldMoveMod + (size OperandSize) + (rd WritableReg) + (ri Reg) + (rn Reg) + (immr UImm6) + (imms UImm6)) + ;; A conditional-select operation. (CSel (rd WritableReg) diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs index 716f8e207233..a50be004affa 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit.rs @@ -417,14 +417,16 @@ fn enc_ccmp_imm(size: OperandSize, rn: Reg, imm: UImm5, nzcv: NZCV, cond: Cond) | nzcv.bits() } -fn enc_bfm( - bfm_op: BfmOp, - size: OperandSize, - rd: Writable, - rn: Reg, - immr: u8, - imms: u8, -) -> u32 { +impl BfmOp { + fn opc(self) -> u8 { + match self { + BfmOp::UBfm => 0b10, + BfmOp::SBfm => 0b00, + } + } +} + +fn enc_bfm(opc: u8, size: OperandSize, rd: Writable, rn: Reg, immr: u8, imms: u8) -> u32 { match size { OperandSize::Size64 => { debug_assert!(immr <= 63); @@ -435,15 +437,11 @@ fn enc_bfm( debug_assert!(imms <= 31); } } - let opc = match bfm_op { - BfmOp::UBfm => 0b10, - BfmOp::SBfm => 0b00, - // Note: BFM (`01`) is intentionally excluded - }; + debug_assert_eq!(opc & 0b11, opc); let n_bit = size.sf_bit(); 0b0_00_100110_0_000000_000000_00000_00000 | size.sf_bit() << 31 - | opc << 29 + | u32::from(opc) << 29 | n_bit << 22 | u32::from(immr) << 16 | u32::from(imms) << 10 @@ -2936,7 +2934,8 @@ impl MachInstEmit for Inst { } else { (BfmOp::UBfm, OperandSize::Size32) }; - sink.put4(enc_bfm(bfm_op, size, rd, rn, 0, from_bits - 1)); + let opc = bfm_op.opc(); + sink.put4(enc_bfm(opc, size, rd, rn, 0, from_bits - 1)); } &Inst::BitfieldMove { size, @@ -2946,7 +2945,19 @@ impl MachInstEmit for Inst { immr, imms, } => { - sink.put4(enc_bfm(bfm_op, size, rd, rn, immr.value(), imms.value())); + let opc = bfm_op.opc(); + sink.put4(enc_bfm(opc, size, rd, rn, immr.value(), imms.value())); + } + &Inst::BitfieldMoveMod { + size, + rd, + ri, + rn, + immr, + imms, + } => { + debug_assert_eq!(rd.to_reg(), ri); + sink.put4(enc_bfm(0b01, size, rd, rn, immr.value(), imms.value())); } &Inst::Jump { ref dest } => { let off = sink.cur_offset(); diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index 2f91ae06f105..c2e35f0b0532 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -802,13 +802,16 @@ fn aarch64_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) { collector.reg_use(rn); } Inst::BitfieldMove { rd, rn, .. } => { - // BFM has been excluded from this instruction format - // as it can leave some bits of `rd` unchanged. - // In contrast, the UBFM and SBFM instructions always - // replace all bits in `rd`, making it a true def. + // The UBFM and SBFM instructions overwrite all bits in `rd`, + // unlike BFM which is represented as `BitfieldMoveMod` instead. collector.reg_def(rd); collector.reg_use(rn); } + Inst::BitfieldMoveMod { rd, ri, rn, .. } => { + collector.reg_reuse_def(rd, 1); // `rd` == `ri`. + collector.reg_use(ri); + collector.reg_use(rn); + } Inst::Args { args } => { for ArgPair { vreg, preg } in args { collector.reg_fixed_def(vreg, *preg); @@ -2627,6 +2630,21 @@ impl Inst { let imms = imms.pretty_print(0); format!("{op} {rd}, {rn}, {immr}, {imms}") } + &Inst::BitfieldMoveMod { + size, + rd, + ri, + rn, + immr, + imms, + } => { + let rd = pretty_print_ireg(rd.to_reg(), size); + let ri = pretty_print_ireg(ri, size); + let rn = pretty_print_ireg(rn, size); + let immr = immr.pretty_print(0); + let imms = imms.pretty_print(0); + format!("bfm {rd}, {ri}, {rn}, {immr}, {imms}") + } &Inst::Call { ref info } => { let try_call = info .try_call_info From c21d5d130f2b755f5046123465700bd81a638e0b Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Wed, 26 Aug 2026 16:58:09 +1000 Subject: [PATCH 6/8] rename `sbfm_imm(r|s)` to `bfm_imm(r|s)`, as it's used for both SBFM and UBFM; add spec clauses to both helpers --- cranelift/codegen/src/isa/aarch64/lower.isle | 22 ++++++++++++++----- .../codegen/src/isa/aarch64/lower/isle.rs | 4 ++-- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 1f992c75021a..3858349d7da7 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1703,16 +1703,26 @@ ;; an appropriate pair of `ishl` and `ushr`/`sshr` operations. (rule sbfm 1 (lower (sshr (ty_32_or_64 ty) (ishl _ x (u64_from_iconst a)) (u64_from_iconst b))) - (bitfield_move ty (BfmOp.SBfm) x (sbfm_immr ty a b) (sbfm_imms ty a b))) + (bitfield_move ty (BfmOp.SBfm) x (bfm_immr ty a b) (bfm_imms ty a b))) (rule ubfm 1 (lower (ushr (ty_32_or_64 ty) (ishl _ x (u64_from_iconst a)) (u64_from_iconst b))) - (bitfield_move ty (BfmOp.UBfm) x (sbfm_immr ty a b) (sbfm_imms ty a b))) + (bitfield_move ty (BfmOp.UBfm) x (bfm_immr ty a b) (bfm_imms ty a b))) ;; Helper methods for constructing the correct `immr` and `imms` immediates. -(decl sbfm_immr (Type u64 u64) UImm6) -(extern constructor sbfm_immr sbfm_immr) -(decl sbfm_imms (Type u64 u64) UImm6) -(extern constructor sbfm_imms sbfm_imms) +(spec (bfm_immr ty a b) + (provide (bvult result #x40)) + (require (bvult a #x40)) + (require (bvult b #x40)) +) +(decl bfm_immr (Type u64 u64) UImm6) +(extern constructor bfm_immr bfm_immr) +(spec (bfm_imms ty a b) + (provide (bvult result #x40)) + (require (bvult a #x40)) + (require (bvult b #x40)) +) +(decl bfm_imms (Type u64 u64) UImm6) +(extern constructor bfm_imms bfm_imms) ;; Shift for i128. (rule (lower (sshr $I128 x y)) diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs index 27354cf8a552..845d101c4c5b 100644 --- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs +++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs @@ -243,7 +243,7 @@ impl Context for IsleContext<'_, '_, MInst, AArch64Backend> { /// Compute the `immr` value for an `sbfm` instruction, /// derived by fusing an `ishl` by amount `a`, with an `sshr` by amount `b`. - fn sbfm_immr(&mut self, ty: Type, a: u64, b: u64) -> UImm6 { + fn bfm_immr(&mut self, ty: Type, a: u64, b: u64) -> UImm6 { let w = ty.lane_bits() as u8; debug_assert!(w <= 64); @@ -255,7 +255,7 @@ impl Context for IsleContext<'_, '_, MInst, AArch64Backend> { /// Compute the `imms` value for an `sbfm` instruction, /// derived by fusing an `ishl` by amount `a`, with an `sshr` by amount `b`. - fn sbfm_imms(&mut self, ty: Type, a: u64, _b: u64) -> UImm6 { + fn bfm_imms(&mut self, ty: Type, a: u64, _b: u64) -> UImm6 { let w = ty.lane_bits() as u8; debug_assert!(w <= 64); From 84028f7a1727cbb732c6a6eca501a23673c02b28 Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Thu, 27 Aug 2026 02:12:26 +1000 Subject: [PATCH 7/8] fix `bfm_immr/imms` spec stubs; prtest:full --- cranelift/codegen/src/isa/aarch64/lower.isle | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 3858349d7da7..677c0540dadd 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1709,18 +1709,10 @@ (bitfield_move ty (BfmOp.UBfm) x (bfm_immr ty a b) (bfm_imms ty a b))) ;; Helper methods for constructing the correct `immr` and `imms` immediates. -(spec (bfm_immr ty a b) - (provide (bvult result #x40)) - (require (bvult a #x40)) - (require (bvult b #x40)) -) +(spec (bfm_immr ty a b) (provide true)) (decl bfm_immr (Type u64 u64) UImm6) (extern constructor bfm_immr bfm_immr) -(spec (bfm_imms ty a b) - (provide (bvult result #x40)) - (require (bvult a #x40)) - (require (bvult b #x40)) -) +(spec (bfm_imms ty a b) (provide true)) (decl bfm_imms (Type u64 u64) UImm6) (extern constructor bfm_imms bfm_imms) From 0abea88e6347eca501ea8212e71eeebbf923f0d5 Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Thu, 27 Aug 2026 12:12:26 +1000 Subject: [PATCH 8/8] mark `BitfieldMove` with TODO tag; prtest:full --- cranelift/codegen/src/isa/aarch64/inst.isle | 1 + 1 file changed, 1 insertion(+) diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index ecc59d2fc27c..d67ed7c8020f 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -1225,6 +1225,7 @@ ;; Leaves currently without specs: excluded from verification via todo. (attr MInst.Udf (tag TODO)) +(attr MInst.BitfieldMove (tag TODO)) ;; An ALU operation. This can be paired with several instruction formats ;; below (see `Inst`) in any combination.