From b1e2c4af02a64525e922263cf1a4ac077143068e Mon Sep 17 00:00:00 2001 From: Alexander Rafferty Date: Fri, 21 Aug 2026 17:41:12 +1000 Subject: [PATCH 1/4] 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/4] 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/4] 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/4] 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 {