Skip to content

Commit d2a22d4

Browse files
authored
winch(aarch64): Handle division signedness (#9784)
This commit fixes how signedness division is hanlded in aarch64. Prior to this commit, sign-extension was emitted unconditionally. This commit ensures that the correct extension is emitted depending on the division kind. Additionally this commit prefers making use of existing assembler helpers.
1 parent bb68f41 commit d2a22d4

File tree

7 files changed

+33
-43
lines changed

7 files changed

+33
-43
lines changed

tests/disas/winch/aarch64/i32_divu/const.wat

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
;; mov x16, #0x14
2424
;; mov w1, w16
2525
;; cbz x0, #0x54
26-
;; 34: sxtw x0, w0
27-
;; sxtw x1, w1
26+
;; 34: mov w0, w0
27+
;; mov w1, w1
2828
;; udiv x1, x1, x0
2929
;; mov w0, w1
3030
;; add sp, sp, #0x10

tests/disas/winch/aarch64/i32_divu/one_zero.wat

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
;; mov x16, #1
2424
;; mov w1, w16
2525
;; cbz x0, #0x54
26-
;; 34: sxtw x0, w0
27-
;; sxtw x1, w1
26+
;; 34: mov w0, w0
27+
;; mov w1, w1
2828
;; udiv x1, x1, x0
2929
;; mov w0, w1
3030
;; add sp, sp, #0x10

tests/disas/winch/aarch64/i32_divu/params.wat

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
;; ldur w0, [x28]
2424
;; ldur w1, [x28, #4]
2525
;; cbz x0, #0x54
26-
;; 34: sxtw x0, w0
27-
;; sxtw x1, w1
26+
;; 34: mov w0, w0
27+
;; mov w1, w1
2828
;; udiv x1, x1, x0
2929
;; mov w0, w1
3030
;; add sp, sp, #0x18

tests/disas/winch/aarch64/i32_divu/signed.wat

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
;; orr x16, xzr, #0xffffffff
2424
;; mov w1, w16
2525
;; cbz x0, #0x54
26-
;; 34: sxtw x0, w0
27-
;; sxtw x1, w1
26+
;; 34: mov w0, w0
27+
;; mov w1, w1
2828
;; udiv x1, x1, x0
2929
;; mov w0, w1
3030
;; add sp, sp, #0x10

tests/disas/winch/aarch64/i32_divu/zero_zero.wat

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
;; mov x16, #0
2424
;; mov w1, w16
2525
;; cbz x0, #0x54
26-
;; 34: sxtw x0, w0
27-
;; sxtw x1, w1
26+
;; 34: mov w0, w0
27+
;; mov w1, w1
2828
;; udiv x1, x1, x0
2929
;; mov w0, w1
3030
;; add sp, sp, #0x10

winch/codegen/src/isa/aarch64/asm.rs

+22-33
Original file line numberDiff line numberDiff line change
@@ -407,23 +407,21 @@ impl Assembler {
407407
kind: DivKind,
408408
size: OperandSize,
409409
) {
410-
// Check for division by 0
411-
self.emit(Inst::TrapIf {
412-
kind: CondBrKind::Zero(divisor.into()),
413-
trap_code: TrapCode::INTEGER_DIVISION_BY_ZERO,
414-
});
410+
// Check for division by 0.
411+
self.trapz(divisor, TrapCode::INTEGER_DIVISION_BY_ZERO);
415412

416413
// check for overflow
417414
if kind == DivKind::Signed {
418-
// we first check whether the divisor is -1
419-
self.emit(Inst::AluRRImm12 {
420-
alu_op: ALUOp::AddS,
421-
size: size.into(),
422-
rd: writable!(zero().into()),
423-
rn: divisor.into(),
424-
imm12: Imm12::maybe_from_u64(1).expect("1 fits in 12 bits"),
425-
});
426-
// if it is -1, then we check if the dividend is MIN
415+
// Check for divisor overflow.
416+
self.emit_alu_rri(
417+
ALUOp::AddS,
418+
Imm12::maybe_from_u64(1).expect("1 to fit in 12 bits"),
419+
divisor,
420+
writable!(zero()),
421+
size,
422+
);
423+
424+
// Check if the dividend is 1.
427425
self.emit(Inst::CCmpImm {
428426
size: size.into(),
429427
rn: dividend.into(),
@@ -432,31 +430,22 @@ impl Assembler {
432430
cond: Cond::Eq,
433431
});
434432

435-
// Finally, trap if the previous operation overflowed
436-
self.emit(Inst::TrapIf {
437-
kind: CondBrKind::Cond(Cond::Vs),
438-
trap_code: TrapCode::INTEGER_OVERFLOW,
439-
})
433+
// Finally, trap if the previous operation overflowed.
434+
self.trapif(Cond::Vs, TrapCode::INTEGER_OVERFLOW);
440435
}
441436

442437
// `cranelift-codegen` doesn't support emitting u/sdiv for anything but I64,
443438
// we therefore sign-extend the operand.
444439
// see: https://github.com/bytecodealliance/wasmtime/issues/9766
445440
if size == OperandSize::S32 {
446-
self.emit(Inst::Extend {
447-
rd: writable!(divisor.into()),
448-
rn: divisor.into(),
449-
signed: true,
450-
from_bits: 32,
451-
to_bits: 64,
452-
});
453-
self.emit(Inst::Extend {
454-
rd: writable!(dividend.into()),
455-
rn: dividend.into(),
456-
signed: true,
457-
from_bits: 32,
458-
to_bits: 64,
459-
});
441+
let extend_kind = if kind == DivKind::Signed {
442+
ExtendKind::I64Extend32S
443+
} else {
444+
ExtendKind::I64ExtendI32U
445+
};
446+
447+
self.extend(divisor, writable!(divisor), extend_kind);
448+
self.extend(dividend, writable!(dividend), extend_kind);
460449
}
461450

462451
let op = match kind {

winch/codegen/src/masm.rs

+1
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ pub(crate) enum ShiftKind {
163163
/// Kinds of extends in WebAssembly. Each MacroAssembler implementation
164164
/// is responsible for emitting the correct sequence of instructions when
165165
/// lowering to machine code.
166+
#[derive(Copy, Clone)]
166167
pub(crate) enum ExtendKind {
167168
/// Sign extends i32 to i64.
168169
I64ExtendI32S,

0 commit comments

Comments
 (0)