Skip to content

Commit

Permalink
Merge pull request #2763 from cfallin/fix-srem-trap
Browse files Browse the repository at this point in the history
Handle `srem` properly when `avoid_div_traps` is false.
  • Loading branch information
cfallin authored Mar 25, 2021
2 parents db7ec95 + b429f77 commit 8636359
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 2 deletions.
1 change: 0 additions & 1 deletion cranelift/codegen/src/isa/x64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,6 @@ pub(crate) fn emit(
// idiv %divisor
//
// $done:
debug_assert!(info.flags().avoid_div_traps());

// Check if the divisor is zero, first.
let inst = Inst::cmp_rmi_r(*size, RegMemImm::imm(0), divisor.to_reg());
Expand Down
3 changes: 2 additions & 1 deletion cranelift/codegen/src/isa/x64/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5181,7 +5181,8 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
input_ty,
));

if flags.avoid_div_traps() {
// Always do explicit checks for `srem`: otherwise, INT_MIN % -1 is not handled properly.
if flags.avoid_div_traps() || op == Opcode::Srem {
// A vcode meta-instruction is used to lower the inline checks, since they embed
// pc-relative offsets that must not change, thus requiring regalloc to not
// interfere by introducing spills and reloads.
Expand Down
12 changes: 12 additions & 0 deletions cranelift/filetests/filetests/isa/x64/div-checks-run.clif
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
test run
set avoid_div_traps=false
target x86_64
feature "experimental_x64"

function %f0(i32, i32) -> i32 {
block0(v0: i32, v1: i32):
v2 = srem.i32 v0, v1
return v2
}

; run: %f0(0x80000000, 0xffffffff) == 0
19 changes: 19 additions & 0 deletions cranelift/filetests/filetests/isa/x64/div-checks.clif
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
test compile
set avoid_div_traps=false
target x86_64
feature "experimental_x64"

;; We should get the checked-div/rem sequence (`srem` pseudoinst below) even
;; when `avoid_div_traps` above is false (i.e. even when the host is normally
;; willing to accept SIGFPEs as Wasm traps). The machine will SIGFPE in some
;; cases when `srem` is valid (specifically -INT_MIN % -1).
function %f0(i64, i64) -> i64 {
block0(v0: i64, v1: i64):
v2 = srem.i64 v0, v1
; check: movq %rdi, %rax
; nextln: movl $$0, %edx
; nextln: srem $$rax:$$rdx, %rsi
; nextln: movq %rdx, %rax

return v2
}

0 comments on commit 8636359

Please sign in to comment.