x64: Take SIGFPE signals for divide traps (#6026)

* x64: Take SIGFPE signals for divide traps

Prior to this commit Wasmtime would configure `avoid_div_traps=true`
unconditionally for Cranelift. This, for the division-based
instructions, would change emitted code to explicitly trap on trap
conditions instead of letting the `div` x86 instruction trap.

There's no specific reason for Wasmtime, however, to specifically avoid
traps in the `div` instruction. This means that the extra generated
branches on x86 aren't necessary since the `div` and `idiv` instructions
already trap for similar conditions as wasm requires.

This commit instead disables the `avoid_div_traps` setting for
Wasmtime's usage of Cranelift. Subsequently the codegen rules were
updated slightly:

* When `avoid_div_traps=true`, traps are no longer emitted for `div`
  instructions.
* The `udiv`/`urem` instructions now list their trap as divide-by-zero
  instead of integer overflow.
* The lowering for `sdiv` was updated to still explicitly check for zero
  but the integer overflow case is deferred to the instruction itself.
* The lowering of `srem` no longer checks for zero and the listed trap
  for the `div` instruction is a divide-by-zero.

This means that the codegen for `udiv` and `urem` no longer have any
branches. The codegen for `sdiv` removes one branch but keeps the
zero-check to differentiate the two kinds of traps. The codegen for
`srem` removes one branch but keeps the -1 check since the semantics of
`srem` mismatch with the semantics of `idiv` with a -1 divisor
(specifically for INT_MIN).

This is unlikely to have really all that much of a speedup but was
something I noticed during #6008 which seemed like it'd be good to clean
up. Plus Wasmtime's signal handling was already set up to catch
`SIGFPE`, it was just never firing.

* Remove the `avoid_div_traps` cranelift setting

With no known users currently removing this should be possible and helps
simplify the x64 backend.

* x64: GC more support for avoid_div_traps

Remove the `validate_sdiv_divisor*` pseudo-instructions and clean up
some of the ISLE rules now that `div` is allowed to itself trap
unconditionally.

* x64: Store div trap code in instruction itself

* Keep divisors in registers, not in memory

Don't accidentally fold multiple traps together

* Handle EXC_ARITHMETIC on macos

* Update emit tests

* Update winch and tests
This commit is contained in:
Alex Crichton
2023-03-15 19:18:45 -05:00
committed by GitHub
parent 5ff2824ebb
commit 5ae8575296
72 changed files with 505 additions and 2624 deletions

View File

@@ -23,12 +23,6 @@ impl<T> IsaBuilder<T> {
pub fn new(lookup: fn(Triple) -> Result<Builder<T>>) -> Self {
let mut flags = settings::builder();
// There are two possible traps for division, and this way
// we get the proper one if code traps.
flags
.enable("avoid_div_traps")
.expect("should be valid flag");
// We don't use probestack as a stack limit mechanism
flags
.set("enable_probestack", "false")

View File

@@ -244,7 +244,7 @@ unsafe fn handle_exception(request: &mut ExceptionRequest) -> bool {
// First make sure that this exception is one that we actually expect to
// get raised by wasm code. All other exceptions we safely ignore.
match request.body.exception as u32 {
EXC_BAD_ACCESS | EXC_BAD_INSTRUCTION => {}
EXC_BAD_ACCESS | EXC_BAD_INSTRUCTION | EXC_ARITHMETIC => {}
_ => return false,
}
@@ -424,7 +424,7 @@ pub fn lazy_per_thread_init() {
let this_thread = mach_thread_self();
let kret = thread_set_exception_ports(
this_thread,
EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION,
EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC,
WASMTIME_PORT,
EXCEPTION_DEFAULT | MACH_EXCEPTION_CODES,
mach_addons::THREAD_STATE_NONE,

View File

@@ -350,7 +350,7 @@ impl Engine {
/// currently rely on the compiler informing us of all settings, including
/// those disabled. Settings then fall in a few buckets:
///
/// * Some settings must be enabled, such as `avoid_div_traps`.
/// * Some settings must be enabled, such as `preserve_frame_pointers`.
/// * Some settings must have a particular value, such as
/// `libcall_call_conv`.
/// * Some settings do not matter as to their value, such as `opt_level`.
@@ -364,7 +364,6 @@ impl Engine {
// These settings must all have be enabled, since their value
// can affect the way the generated code performs or behaves at
// runtime.
"avoid_div_traps" => *value == FlagValue::Bool(true),
"libcall_call_conv" => *value == FlagValue::Enum("isa_default".into()),
"preserve_frame_pointers" => *value == FlagValue::Bool(true),
"enable_probestack" => *value == FlagValue::Bool(crate::config::probestack_supported(target.architecture)),

View File

@@ -489,9 +489,10 @@ mod test {
let engine = Engine::default();
let mut metadata = Metadata::new(&engine);
metadata
.shared_flags
.insert("avoid_div_traps".to_string(), FlagValue::Bool(false));
metadata.shared_flags.insert(
"preserve_frame_pointers".to_string(),
FlagValue::Bool(false),
);
match metadata.check_compatible(&engine) {
Ok(_) => unreachable!(),
@@ -500,7 +501,7 @@ mod test {
compilation settings of module incompatible with native host
Caused by:
setting \"avoid_div_traps\" is configured to Bool(false) which is not supported"
setting \"preserve_frame_pointers\" is configured to Bool(false) which is not supported"
)),
}