Skip to content

Commit

Permalink
aarch64: support udiv for 32bit integers (#9798)
Browse files Browse the repository at this point in the history 
* emit 32bit udiv

* winch: aarch64 udiv/urem without extension

* remove stray dbg!

* fmt

* remove println

* fix formatting in ISLE

* Sized TrapIf

* move operand size into CondBrKind variant

* show_reg_sized fallback
  • Loading branch information
@MarinPostma
MarinPostma authored Dec 17, 2024
1 parent b2f160c commit 031a28a
Show file tree
Hide file tree
Showing 32 changed files with 177 additions and 181 deletions.
24 changes: 12 additions & 12 deletions cranelift/codegen/src/isa/aarch64/inst.isle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1992,10 +1992,10 @@
(decl nzcv (bool bool bool bool) NZCV)
(extern constructor nzcv nzcv)

(decl cond_br_zero (Reg) CondBrKind)
(decl cond_br_zero (Reg OperandSize) CondBrKind)
(extern constructor cond_br_zero cond_br_zero)

(decl cond_br_not_zero (Reg) CondBrKind)
(decl cond_br_not_zero (Reg OperandSize) CondBrKind)
(extern constructor cond_br_not_zero cond_br_not_zero)

(decl cond_br_cond (Cond) CondBrKind)
Expand Down Expand Up @@ -3514,19 +3514,19 @@
Zero
NonZero))

(decl zero_cond_to_cond_br (ZeroCond Reg) CondBrKind)
(rule (zero_cond_to_cond_br (ZeroCond.Zero) reg)
(cond_br_zero reg))
(decl zero_cond_to_cond_br (ZeroCond Reg OperandSize) CondBrKind)
(rule (zero_cond_to_cond_br (ZeroCond.Zero) reg size)
(cond_br_zero reg size))

(rule (zero_cond_to_cond_br (ZeroCond.NonZero) reg)
(cond_br_not_zero reg))
(rule (zero_cond_to_cond_br (ZeroCond.NonZero) reg size)
(cond_br_not_zero reg size))

(decl trap_if_val (ZeroCond Value TrapCode) InstOutput)
(rule (trap_if_val zero_cond val @ (value_type (fits_in_64 _)) trap_code)
(let ((reg Reg (put_in_reg_zext64 val)))
(side_effect
(SideEffectNoResult.Inst
(MInst.TrapIf (zero_cond_to_cond_br zero_cond reg) trap_code)))))
(MInst.TrapIf (zero_cond_to_cond_br zero_cond reg (operand_size $I64)) trap_code)))))

(rule -1 (trap_if_val zero_cond val @ (value_type $I128) trap_code)
(let ((c ValueRegs (put_in_regs val))
Expand All @@ -3535,7 +3535,7 @@
(c_test Reg (orr $I64 c_lo c_hi)))
(side_effect
(SideEffectNoResult.Inst
(MInst.TrapIf (zero_cond_to_cond_br zero_cond c_test) trap_code)))))
(MInst.TrapIf (zero_cond_to_cond_br zero_cond c_test (operand_size $I64)) trap_code)))))

;; Immediate value helpers ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Expand Down Expand Up @@ -3659,9 +3659,9 @@

;; Misc instruction helpers ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(decl trap_if_zero_divisor (Reg) Reg)
(rule (trap_if_zero_divisor reg)
(let ((_ Unit (emit (MInst.TrapIf (cond_br_zero reg) (trap_code_division_by_zero)))))
(decl trap_if_zero_divisor (Reg OperandSize) Reg)
(rule (trap_if_zero_divisor reg size)
(let ((_ Unit (emit (MInst.TrapIf (cond_br_zero reg size ) (trap_code_division_by_zero)))))
reg))

(decl size_from_ty (Type) OperandSize)
Expand Down
8 changes: 4 additions & 4 deletions cranelift/codegen/src/isa/aarch64/inst/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,9 @@ impl Cond {
#[derive(Clone, Copy, Debug)]
pub enum CondBrKind {
/// Condition: given register is zero.
Zero(Reg),
Zero(Reg, OperandSize),
/// Condition: given register is nonzero.
NotZero(Reg),
NotZero(Reg, OperandSize),
/// Condition: the given condition-code test is true.
Cond(Cond),
}
Expand All @@ -244,8 +244,8 @@ impl CondBrKind {
/// Return the inverted branch condition.
pub fn invert(self) -> CondBrKind {
match self {
CondBrKind::Zero(reg) => CondBrKind::NotZero(reg),
CondBrKind::NotZero(reg) => CondBrKind::Zero(reg),
CondBrKind::Zero(reg, size) => CondBrKind::NotZero(reg, size),
CondBrKind::NotZero(reg, size) => CondBrKind::Zero(reg, size),
CondBrKind::Cond(c) => CondBrKind::Cond(c.invert()),
}
}
Expand Down
25 changes: 18 additions & 7 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,21 @@ fn enc_cbr(op_31_24: u32, off_18_0: u32, op_4: u32, cond: u32) -> u32 {
(op_31_24 << 24) | (off_18_0 << 5) | (op_4 << 4) | cond
}

/// Set the size bit of an instruction.
fn enc_op_size(op: u32, size: OperandSize) -> u32 {
(op & !(1 << 31)) | (size.sf_bit() << 31)
}

fn enc_conditional_br(taken: BranchTarget, kind: CondBrKind) -> u32 {
match kind {
CondBrKind::Zero(reg) => enc_cmpbr(0b1_011010_0, taken.as_offset19_or_zero(), reg),
CondBrKind::NotZero(reg) => enc_cmpbr(0b1_011010_1, taken.as_offset19_or_zero(), reg),
CondBrKind::Zero(reg, size) => enc_op_size(
enc_cmpbr(0b0_011010_0, taken.as_offset19_or_zero(), reg),
size,
),
CondBrKind::NotZero(reg, size) => enc_op_size(
enc_cmpbr(0b0_011010_1, taken.as_offset19_or_zero(), reg),
size,
),
CondBrKind::Cond(c) => enc_cbr(0b01010100, taken.as_offset19_or_zero(), 0b0, c.bits()),
}
}
Expand Down Expand Up @@ -728,8 +739,7 @@ impl MachInstEmit for Inst {
rm,
} => {
debug_assert!(match alu_op {
ALUOp::SDiv | ALUOp::UDiv | ALUOp::SMulH | ALUOp::UMulH =>
size == OperandSize::Size64,
ALUOp::SMulH | ALUOp::UMulH => size == OperandSize::Size64,
_ => true,
});
let top11 = match alu_op {
Expand All @@ -749,11 +759,12 @@ impl MachInstEmit for Inst {
ALUOp::AddS => 0b00101011_000,
ALUOp::SubS => 0b01101011_000,
ALUOp::SDiv => 0b10011010_110,
ALUOp::UDiv => 0b10011010_110,
ALUOp::UDiv => 0b00011010_110,
ALUOp::RotR | ALUOp::Lsr | ALUOp::Asr | ALUOp::Lsl => 0b00011010_110,
ALUOp::SMulH => 0b10011011_010,
ALUOp::UMulH => 0b10011011_110,
};

let top11 = top11 | size.sf_bit() << 10;
let bit15_10 = match alu_op {
ALUOp::SDiv => 0b000011,
Expand Down Expand Up @@ -1612,7 +1623,7 @@ impl MachInstEmit for Inst {
let br_offset = sink.cur_offset();
sink.put4(enc_conditional_br(
BranchTarget::Label(again_label),
CondBrKind::NotZero(x24),
CondBrKind::NotZero(x24, OperandSize::Size64),
));
sink.use_label_at_offset(br_offset, again_label, LabelUse::Branch19);
}
Expand Down Expand Up @@ -1705,7 +1716,7 @@ impl MachInstEmit for Inst {
let br_again_offset = sink.cur_offset();
sink.put4(enc_conditional_br(
BranchTarget::Label(again_label),
CondBrKind::NotZero(x24),
CondBrKind::NotZero(x24, OperandSize::Size64),
));
sink.use_label_at_offset(br_again_offset, again_label, LabelUse::Branch19);

Expand Down
4 changes: 2 additions & 2 deletions cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5902,15 +5902,15 @@ fn test_aarch64_binemit() {
insns.push((
Inst::TrapIf {
trap_code: TrapCode::STACK_OVERFLOW,
kind: CondBrKind::NotZero(xreg(8)),
kind: CondBrKind::NotZero(xreg(8), OperandSize::Size64),
},
"280000B51FC10000",
"cbnz x8, #trap=stk_ovf",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::STACK_OVERFLOW,
kind: CondBrKind::Zero(xreg(8)),
kind: CondBrKind::Zero(xreg(8), OperandSize::Size64),
},
"280000B41FC10000",
"cbz x8, #trap=stk_ovf",
Expand Down
20 changes: 10 additions & 10 deletions cranelift/codegen/src/isa/aarch64/inst/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ fn aarch64_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) {
}
}
Inst::CondBr { kind, .. } => match kind {
CondBrKind::Zero(rt) | CondBrKind::NotZero(rt) => collector.reg_use(rt),
CondBrKind::Zero(rt, _) | CondBrKind::NotZero(rt, _) => collector.reg_use(rt),
CondBrKind::Cond(_) => {}
},
Inst::TestBitAndBranch { rn, .. } => {
Expand All @@ -886,7 +886,7 @@ fn aarch64_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) {
Inst::Brk => {}
Inst::Udf { .. } => {}
Inst::TrapIf { kind, .. } => match kind {
CondBrKind::Zero(rt) | CondBrKind::NotZero(rt) => collector.reg_use(rt),
CondBrKind::Zero(rt, _) | CondBrKind::NotZero(rt, _) => collector.reg_use(rt),
CondBrKind::Cond(_) => {}
},
Inst::Adr { rd, .. } | Inst::Adrp { rd, .. } => {
Expand Down Expand Up @@ -2632,12 +2632,12 @@ impl Inst {
let taken = taken.pretty_print(0);
let not_taken = not_taken.pretty_print(0);
match kind {
&CondBrKind::Zero(reg) => {
let reg = pretty_print_reg(reg);
&CondBrKind::Zero(reg, size) => {
let reg = pretty_print_reg_sized(reg, size);
format!("cbz {reg}, {taken} ; b {not_taken}")
}
&CondBrKind::NotZero(reg) => {
let reg = pretty_print_reg(reg);
&CondBrKind::NotZero(reg, size) => {
let reg = pretty_print_reg_sized(reg, size);
format!("cbnz {reg}, {taken} ; b {not_taken}")
}
&CondBrKind::Cond(c) => {
Expand Down Expand Up @@ -2672,12 +2672,12 @@ impl Inst {
ref kind,
trap_code,
} => match kind {
&CondBrKind::Zero(reg) => {
let reg = pretty_print_reg(reg);
&CondBrKind::Zero(reg, size) => {
let reg = pretty_print_reg_sized(reg, size);
format!("cbz {reg}, #trap={trap_code}")
}
&CondBrKind::NotZero(reg) => {
let reg = pretty_print_reg(reg);
&CondBrKind::NotZero(reg, size) => {
let reg = pretty_print_reg_sized(reg, size);
format!("cbnz {reg}, #trap={trap_code}")
}
&CondBrKind::Cond(c) => {
Expand Down
12 changes: 12 additions & 0 deletions cranelift/codegen/src/isa/aarch64/inst/regs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,18 @@ pub fn pretty_print_reg(reg: Reg) -> String {
show_reg(reg)
}

fn show_reg_sized(reg: Reg, size: OperandSize) -> String {
match reg.class() {
RegClass::Int => show_ireg_sized(reg, size),
RegClass::Float => show_reg(reg),
RegClass::Vector => unreachable!(),
}
}

pub fn pretty_print_reg_sized(reg: Reg, size: OperandSize) -> String {
show_reg_sized(reg, size)
}

/// If `ireg` denotes an Int-classed reg, make a best-effort attempt to show
/// its name at the 32-bit size.
pub fn show_ireg_sized(reg: Reg, size: OperandSize) -> String {
Expand Down
36 changes: 26 additions & 10 deletions cranelift/codegen/src/isa/aarch64/lower.isle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1028,21 +1028,37 @@

;;;; Rules for `udiv` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; TODO: Add UDiv32 to implement 32-bit directly, rather
;; than extending the input.
;;
;; Note that aarch64's `udiv` doesn't trap so to respect the semantics of
;; CLIF's `udiv` the check for zero needs to be manually performed.
(rule udiv (lower (has_type (fits_in_64 ty) (udiv x y)))
(a64_udiv $I64 (put_in_reg_zext64 x) (put_nonzero_in_reg_zext64 y)))

(rule udiv 1 (lower (has_type $I64 (udiv x y)))
(a64_udiv $I64 (put_in_reg x) (put_nonzero_in_reg y)))

(rule udiv (lower (has_type (fits_in_32 ty) (udiv x y)))
(a64_udiv $I32 (put_in_reg_zext32 x) (put_nonzero_in_reg y)))

;; helpers for udiv:
;; Helper for placing a `Value` into a `Reg` and validating that it's nonzero.
(decl put_nonzero_in_reg (Value) Reg)

;; Special case where if a `Value` is known to be nonzero we can trivially
;; move it into a register.
(rule (put_nonzero_in_reg (and (value_type ty) (iconst (nonzero_u64_from_imm64 n))))
(imm ty (ImmExtend.Zero) n))

(rule -1 (put_nonzero_in_reg (and (value_type $I64) val))
(trap_if_zero_divisor (put_in_reg val) (operand_size $I64)))

(rule -2 (put_nonzero_in_reg (and (value_type (fits_in_32 _)) val))
(trap_if_zero_divisor (put_in_reg_zext32 val) (operand_size $I32)))

;; Helper for placing a `Value` into a `Reg` and validating that it's nonzero and extending it to 64 bits.
(spec (put_nonzero_in_reg_zext64 x)
(provide (= result (zero_ext 64 x)))
(require (not (= result #x0000000000000000))))
(decl put_nonzero_in_reg_zext64 (Value) Reg)
(rule -1 (put_nonzero_in_reg_zext64 val)
(trap_if_zero_divisor (put_in_reg_zext64 val)))
(rule -1 (put_nonzero_in_reg_zext64 (and (value_type ty) val))
(trap_if_zero_divisor (put_in_reg_zext64 val) (operand_size ty)))

;; Special case where if a `Value` is known to be nonzero we can trivially
;; move it into a register.
Expand Down Expand Up @@ -1092,7 +1108,7 @@
(require (not (= #x0000000000000000 result))))
(decl put_nonzero_in_reg_sext64 (Value) Reg)
(rule -1 (put_nonzero_in_reg_sext64 val)
(trap_if_zero_divisor (put_in_reg_sext64 val)))
(trap_if_zero_divisor (put_in_reg_sext64 val) (operand_size $I64)))

;; Note that this has a special case where if the `Value` is a constant that's
;; not zero we can skip the zero check.
Expand Down Expand Up @@ -3079,14 +3095,14 @@
(rt Reg (orr $I64 c_lo c_hi)))
(emit_side_effect
(with_flags_side_effect flags
(cond_br taken not_taken (cond_br_not_zero rt))))))
(cond_br taken not_taken (cond_br_not_zero rt (operand_size $I64)))))))
(rule -2 (lower_branch (brif c @ (value_type ty) _ _) (two_targets taken not_taken))
(if (ty_int_ref_scalar_64 ty))
(let ((flags ProducesFlags (flags_to_producesflags c))
(rt Reg (put_in_reg_zext64 c)))
(emit_side_effect
(with_flags_side_effect flags
(cond_br taken not_taken (cond_br_not_zero rt))))))
(cond_br taken not_taken (cond_br_not_zero rt (operand_size $I64)))))))

;; Special lowerings for `tbnz` - "Test bit and Branch if Nonzero"
(rule 1 (lower_branch (brif (band x @ (value_type ty) (u64_from_iconst n)) _ _)
Expand Down
8 changes: 4 additions & 4 deletions cranelift/codegen/src/isa/aarch64/lower/isle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,12 +354,12 @@ impl Context for IsleContext<'_, '_, MInst, AArch64Backend> {
self.lower_ctx.emit(inst.clone());
}

fn cond_br_zero(&mut self, reg: Reg) -> CondBrKind {
CondBrKind::Zero(reg)
fn cond_br_zero(&mut self, reg: Reg, size: &OperandSize) -> CondBrKind {
CondBrKind::Zero(reg, *size)
}

fn cond_br_not_zero(&mut self, reg: Reg) -> CondBrKind {
CondBrKind::NotZero(reg)
fn cond_br_not_zero(&mut self, reg: Reg, size: &OperandSize) -> CondBrKind {
CondBrKind::NotZero(reg, *size)
}

fn cond_br_cond(&mut self, cond: &Cond) -> CondBrKind {
Expand Down
Loading

0 comments on commit 031a28a

Please # to comment.