Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x64: Take SIGFPE signals for divide traps #6026

Merged
merged 8 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions cranelift/codegen/meta/src/shared/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,6 @@ pub(crate) fn define() -> SettingGroup {
false,
);

settings.add_bool(
"avoid_div_traps",
"Generate explicit checks around native division instructions to avoid their trapping.",
r#"
Generate explicit checks around native division instructions to
avoid their trapping.

On ISAs like ARM where the native division instructions don't trap,
this setting has no effect - explicit checks are always inserted.
"#,
false,
);

settings.add_bool(
"enable_float",
"Enable the use of floating-point instructions.",
Expand Down
4 changes: 0 additions & 4 deletions cranelift/codegen/src/isa/s390x/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -1446,10 +1446,6 @@
(decl vxrs_ext2_disabled () Type)
(extern extractor vxrs_ext2_disabled vxrs_ext2_disabled)

(decl allow_div_traps () Type)
(extern extractor allow_div_traps allow_div_traps)


;; Helpers for SIMD lane number operations ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; There are two ways to map vector types onto the SIMD vector registers
Expand Down
39 changes: 2 additions & 37 deletions cranelift/codegen/src/isa/s390x/lower.isle
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,6 @@
(rule (lower (has_type (fits_in_64 ty) (udiv x y)))
(let (;; Look at the divisor to determine whether we need to generate
;; an explicit division-by zero check.
(DZcheck bool (zero_divisor_check_needed y))
;; Load up the dividend, by loading the input (possibly zero-
;; extended) input into the low half of the register pair,
;; and setting the high half to zero.
Expand All @@ -545,10 +544,6 @@
;; Load up the divisor, zero-extended if necessary.
(ext_y Reg (put_in_reg_zext32 y))
(ext_ty Type (ty_ext32 ty))
;; Now actually perform the division-by zero check if necessary.
;; This cannot be done earlier than here, because the check
;; requires an already extended divisor value.
(_ Reg (maybe_trap_if_zero_divisor DZcheck ext_ty ext_y))
;; Emit the actual divide instruction.
(pair RegPair (udivmod ext_ty ext_x ext_y)))
;; The quotient can be found in the low half of the result.
Expand All @@ -557,38 +552,13 @@
;; Implement `urem`. Same as `udiv`, but finds the remainder in
;; the high half of the result register pair instead.
(rule (lower (has_type (fits_in_64 ty) (urem x y)))
(let ((DZcheck bool (zero_divisor_check_needed y))
(ext_x RegPair (regpair (imm (ty_ext32 ty) 0)
(let ((ext_x RegPair (regpair (imm (ty_ext32 ty) 0)
(put_in_reg_zext32 x)))
(ext_y Reg (put_in_reg_zext32 y))
(ext_ty Type (ty_ext32 ty))
(_ Reg (maybe_trap_if_zero_divisor DZcheck ext_ty ext_y))
(pair RegPair (udivmod ext_ty ext_x ext_y)))
(copy_reg ty (regpair_hi pair))))

;; Determine whether we need to perform a divide-by-zero-check.
;;
;; If the `avoid_div_traps` flag is false, we never need to perform
;; that check; we can rely on the divide instruction itself to trap.
;;
;; If the `avoid_div_traps` flag is true, we perform the check explicitly.
;; This still can be omittted if the divisor is a non-zero immediate.
(decl zero_divisor_check_needed (Value) bool)
(rule 2 (zero_divisor_check_needed (i64_from_value x))
(if (i64_nonzero x))
$false)
(rule 1 (zero_divisor_check_needed (value_type (allow_div_traps))) $false)
(rule 0 (zero_divisor_check_needed _) $true)

;; Perform the divide-by-zero check if required.
;; This is simply a compare-and-trap of the (extended) divisor against 0.
(decl maybe_trap_if_zero_divisor (bool Type Reg) Reg)
(rule (maybe_trap_if_zero_divisor $false _ _) (invalid_reg))
(rule (maybe_trap_if_zero_divisor $true ext_ty reg)
(icmps_simm16_and_trap ext_ty reg 0
(intcc_as_cond (IntCC.Equal))
(trap_code_division_by_zero)))


;;;; Rules for `sdiv` and `srem` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Expand All @@ -610,15 +580,12 @@
(rule (lower (has_type (fits_in_64 ty) (sdiv x y)))
(let (;; Look at the divisor to determine whether we need to generate
;; explicit division-by-zero and/or integer-overflow checks.
(DZcheck bool (zero_divisor_check_needed y))
(OFcheck bool (div_overflow_check_needed y))
;; Load up the dividend (sign-extended to 64-bit)
(ext_x Reg (put_in_reg_sext64 x))
;; Load up the divisor (sign-extended if necessary).
(ext_y Reg (put_in_reg_sext32 y))
(ext_ty Type (ty_ext32 ty))
;; Perform division-by-zero check (same as for `udiv`).
(_ Reg (maybe_trap_if_zero_divisor DZcheck ext_ty ext_y))
;; Perform integer-overflow check if necessary.
(_ Reg (maybe_trap_if_sdiv_overflow OFcheck ext_ty ty ext_x ext_y))
;; Emit the actual divide instruction.
Expand All @@ -630,12 +597,10 @@
;; the high half of the result register pair instead. Also, handle
;; the integer overflow case differently, see below.
(rule (lower (has_type (fits_in_64 ty) (srem x y)))
(let ((DZcheck bool (zero_divisor_check_needed y))
(OFcheck bool (div_overflow_check_needed y))
(let ((OFcheck bool (div_overflow_check_needed y))
(ext_x Reg (put_in_reg_sext64 x))
(ext_y Reg (put_in_reg_sext32 y))
(ext_ty Type (ty_ext32 ty))
(_ Reg (maybe_trap_if_zero_divisor DZcheck ext_ty ext_y))
(checked_x Reg (maybe_avoid_srem_overflow OFcheck ext_ty ext_x ext_y))
(pair RegPair (sdivmod ext_ty checked_x ext_y)))
(copy_reg ty (regpair_hi pair))))
Expand Down
9 changes: 0 additions & 9 deletions cranelift/codegen/src/isa/s390x/lower/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,6 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, S390xBackend> {
Box::new(symbol_reloc.clone())
}

#[inline]
fn allow_div_traps(&mut self, _: Type) -> Option<()> {
if !self.backend.flags.avoid_div_traps() {
Some(())
} else {
None
}
}

#[inline]
fn mie2_enabled(&mut self, _: Type) -> Option<()> {
if self.backend.isa_flags.has_mie2() {
Expand Down
64 changes: 14 additions & 50 deletions cranelift/codegen/src/isa/x64/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
;; instruction.
(Div (size OperandSize) ;; 2, 4, or 8
(sign DivSignedness)
(trap TrapCode)
(divisor GprMem)
(dividend_lo Gpr)
(dividend_hi Gpr)
Expand All @@ -71,6 +72,7 @@

;; Same as `Div`, but for 8-bits where the regalloc behavior is different
(Div8 (sign DivSignedness)
(trap TrapCode)
(divisor GprMem)
(dividend Gpr)
(dst WritableGpr))
Expand Down Expand Up @@ -103,29 +105,6 @@
(divisor Gpr)
(dst WritableGpr))

;; Validates that the `divisor` can be safely divided into the
;; `dividend`.
;;
;; This is a separate pseudo-instruction because it has some jumps in
;; ways that can't be modeled otherwise with instructions right now. This
;; will trap if the `divisor` is zero or if it's -1 and `dividend` is
;; INT_MIN for the associated type.
;;
;; Note that 64-bit types must use `ValidateSdivDivisor64`.
(ValidateSdivDivisor (size OperandSize)
(dividend Gpr)
(divisor Gpr))

;; Same as `ValidateSdivDivisor` but for 64-bit types.
;;
;; This is a distinct instruction because the emission in `emit.rs`
;; requires a temporary register to load an immediate into, hence the
;; `tmp` field in this instruction not present in the non-64-bit one.
(ValidateSdivDivisor64 (dividend Gpr)
(divisor Gpr)
(tmp WritableGpr))


;; Do a sign-extend based on the sign of the value in rax into rdx: (cwd
;; cdq cqo) or al into ah: (cbw)
(SignExtendData (size OperandSize) ;; 1, 2, 4, or 8
Expand Down Expand Up @@ -4571,32 +4550,32 @@
dst))

;; Helper for creating `Div8` instructions
(decl x64_div8 (Gpr GprMem DivSignedness) Gpr)
(rule (x64_div8 dividend divisor sign)
(decl x64_div8 (Gpr GprMem DivSignedness TrapCode) Gpr)
(rule (x64_div8 dividend divisor sign trap)
(let ((dst WritableGpr (temp_writable_gpr))
(_ Unit (emit (MInst.Div8 sign divisor dividend dst))))
(_ Unit (emit (MInst.Div8 sign trap divisor dividend dst))))
dst))

;; Helper for creating `Div` instructions
;;
;; Two registers are returned through `ValueRegs` where the first is the
;; quotient and the second is the remainder.
(decl x64_div (Gpr Gpr GprMem OperandSize DivSignedness) ValueRegs)
(rule (x64_div dividend_lo dividend_hi divisor size sign)
(decl x64_div (Gpr Gpr GprMem OperandSize DivSignedness TrapCode) ValueRegs)
(rule (x64_div dividend_lo dividend_hi divisor size sign trap)
(let ((dst_quotient WritableGpr (temp_writable_gpr))
(dst_remainder WritableGpr (temp_writable_gpr))
(_ Unit (emit (MInst.Div size sign divisor dividend_lo dividend_hi dst_quotient dst_remainder))))
(_ Unit (emit (MInst.Div size sign trap divisor dividend_lo dividend_hi dst_quotient dst_remainder))))
(value_regs dst_quotient dst_remainder)))

;; Helper for `Div`, returning the quotient and discarding the remainder.
(decl x64_div_quotient (Gpr Gpr GprMem OperandSize DivSignedness) ValueRegs)
(rule (x64_div_quotient dividend_lo dividend_hi divisor size sign)
(value_regs_get (x64_div dividend_lo dividend_hi divisor size sign) 0))
(decl x64_div_quotient (Gpr Gpr GprMem OperandSize DivSignedness TrapCode) ValueRegs)
(rule (x64_div_quotient dividend_lo dividend_hi divisor size sign trap)
(value_regs_get (x64_div dividend_lo dividend_hi divisor size sign trap) 0))

;; Helper for `Div`, returning the remainder and discarding the quotient.
(decl x64_div_remainder (Gpr Gpr GprMem OperandSize DivSignedness) ValueRegs)
(rule (x64_div_remainder dividend_lo dividend_hi divisor size sign)
(value_regs_get (x64_div dividend_lo dividend_hi divisor size sign) 1))
(decl x64_div_remainder (Gpr Gpr GprMem OperandSize DivSignedness TrapCode) ValueRegs)
(rule (x64_div_remainder dividend_lo dividend_hi divisor size sign trap)
(value_regs_get (x64_div dividend_lo dividend_hi divisor size sign trap) 1))

;; Helper for creating `SignExtendData` instructions
(decl x64_sign_extend_data (Gpr OperandSize) Gpr)
Expand All @@ -4605,21 +4584,6 @@
(_ Unit (emit (MInst.SignExtendData size src dst))))
dst))

;; Helper for creating `ValidateSdivDivisor` instructions.
(decl validate_sdiv_divisor (OperandSize Gpr Gpr) Gpr)
(rule (validate_sdiv_divisor size dividend divisor)
(let ((_ Unit (emit (MInst.ValidateSdivDivisor size dividend divisor))))
divisor))

;; Helper for creating `ValidateSdivDivisor64` instructions.
(decl validate_sdiv_divisor64 (Gpr Gpr) Gpr)
(rule (validate_sdiv_divisor64 dividend divisor)
(let (
(tmp WritableGpr (temp_writable_gpr))
(_ Unit (emit (MInst.ValidateSdivDivisor64 dividend divisor tmp)))
)
divisor))

;;;; Pinned Register ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(decl read_pinned_gpr () Gpr)
Expand Down
66 changes: 15 additions & 51 deletions cranelift/codegen/src/isa/x64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,18 @@ pub(crate) fn emit(
emit_std_enc_enc(sink, prefix, opcode, 1, subopcode, enc_src, rex_flags)
}

Inst::Div { sign, divisor, .. } | Inst::Div8 { sign, divisor, .. } => {
Inst::Div {
sign,
trap,
divisor,
..
}
| Inst::Div8 {
sign,
trap,
divisor,
..
} => {
let divisor = divisor.clone().to_reg_mem().with_allocs(allocs);
let size = match inst {
Inst::Div {
Expand Down Expand Up @@ -437,7 +448,7 @@ pub(crate) fn emit(
OperandSize::Size64 => (0xF7, LegacyPrefixes::None),
};

sink.add_trap(TrapCode::IntegerDivisionByZero);
sink.add_trap(*trap);

let subopcode = match sign {
DivSignedness::Signed => 7,
Expand Down Expand Up @@ -612,13 +623,15 @@ pub(crate) fn emit(
let inst = match size {
OperandSize::Size8 => Inst::div8(
DivSignedness::Signed,
TrapCode::IntegerDivisionByZero,
RegMem::reg(divisor),
Gpr::new(regs::rax()).unwrap(),
Writable::from_reg(Gpr::new(regs::rax()).unwrap()),
),
_ => Inst::div(
size,
DivSignedness::Signed,
TrapCode::IntegerDivisionByZero,
RegMem::reg(divisor),
Gpr::new(regs::rax()).unwrap(),
Gpr::new(regs::rdx()).unwrap(),
Expand All @@ -631,55 +644,6 @@ pub(crate) fn emit(
sink.bind_label(done_label);
}

Inst::ValidateSdivDivisor {
dividend, divisor, ..
}
| Inst::ValidateSdivDivisor64 {
dividend, divisor, ..
} => {
let orig_inst = &inst;
let divisor = allocs.next(divisor.to_reg());
let dividend = allocs.next(dividend.to_reg());
let size = match inst {
Inst::ValidateSdivDivisor { size, .. } => *size,
_ => OperandSize::Size64,
};

// First trap if the divisor is zero
let inst = Inst::cmp_rmi_r(size, RegMemImm::imm(0), divisor);
inst.emit(&[], sink, info, state);
let inst = Inst::trap_if(CC::Z, TrapCode::IntegerDivisionByZero);
inst.emit(&[], sink, info, state);

// Now check if the divisor is -1. If it is then additionally
// check if the dividend is INT_MIN. If it isn't then jump to the
// end. If both conditions here are true then trap.
let inst = Inst::cmp_rmi_r(size, RegMemImm::imm(0xffffffff), divisor);
inst.emit(&[], sink, info, state);
let done = sink.get_label();
one_way_jmp(sink, CC::NZ, done);
let int_min = match orig_inst {
Inst::ValidateSdivDivisor64 { tmp, .. } => {
let tmp = allocs.next(tmp.to_reg().to_reg());
let inst = Inst::imm(size, i64::MIN as u64, Writable::from_reg(tmp));
inst.emit(&[], sink, info, state);
RegMemImm::reg(tmp)
}
_ => RegMemImm::imm(match size {
OperandSize::Size8 => 0x80,
OperandSize::Size16 => 0x8000,
OperandSize::Size32 => 0x80000000,
OperandSize::Size64 => unreachable!(),
}),
};
let inst = Inst::cmp_rmi_r(size, int_min, dividend);
inst.emit(&[], sink, info, state);
let inst = Inst::trap_if(CC::Z, TrapCode::IntegerOverflow);
inst.emit(&[], sink, info, state);

sink.bind_label(done);
}

Inst::Imm {
dst_size,
simm64,
Expand Down
Loading