From ff73c5d71b441c906f13faa8fa8ecc5ba2d18d2f Mon Sep 17 00:00:00 2001 From: Nick Lewycky Date: Tue, 26 Nov 2019 12:15:26 -0800 Subject: [PATCH] Address review feedback from Mark. Fix a bug in Operator::Select and add a comment to explain the intention. Use derived default for ExtraInfo. Make ExtraInfo associated functions const. Turn two asserts into debug_asserts. --- CHANGELOG.md | 2 +- lib/llvm-backend/src/code.rs | 6 +++++- lib/llvm-backend/src/state.rs | 38 +++++++++++++++++++---------------- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30af3c6eb78..aafd7885b72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - [#990](https://github.com/wasmerio/wasmer/pull/990) Default wasmer CLI to `run`. Wasmer will now attempt to parse unrecognized command line options as if they were applied to the run command: `wasmer mywasm.wasm --dir=.` now works! - [#987](https://github.com/wasmerio/wasmer/pull/987) Fix `runtime-c-api` header files when compiled by gnuc. - [#957](https://github.com/wasmerio/wasmer/pull/957) Change the meaning of `wasmer_wasi::is_wasi_module` to detect any type of WASI module, add support for new wasi snapshot_preview1 +- [#934](https://github.com/wasmerio/wasmer/pull/934) Simplify float expressions in the LLVM backend. ## 0.10.2 - 2019-11-18 @@ -36,7 +37,6 @@ Special thanks to [@newpavlov](https://github.com/newpavlov) and [@Maxgy](https: - [#939](https://github.com/wasmerio/wasmer/pull/939) Fix bug causing attempts to append to files with WASI to delete the contents of the file - [#940](https://github.com/wasmerio/wasmer/pull/940) Update supported Rust version to 1.38+ - [#923](https://github.com/wasmerio/wasmer/pull/923) Fix memory leak in the C API caused by an incorrect cast in `wasmer_trampoline_buffer_destroy` -- [#934](https://github.com/wasmerio/wasmer/pull/934) Simplify float expressions in the LLVM backend. - [#921](https://github.com/wasmerio/wasmer/pull/921) In LLVM backend, annotate all memory accesses with TBAA metadata. - [#883](https://github.com/wasmerio/wasmer/pull/883) Allow floating point operations to have arbitrary inputs, even including SNaNs. - [#856](https://github.com/wasmerio/wasmer/pull/856) Expose methods in the runtime C API to get a WASI import object diff --git a/lib/llvm-backend/src/code.rs b/lib/llvm-backend/src/code.rs index 3ecad44e7d5..d60ec5aea95 100644 --- a/lib/llvm-backend/src/code.rs +++ b/lib/llvm-backend/src/code.rs @@ -1740,7 +1740,11 @@ impl FunctionCodeGenerator for LLVMFunctionCodeGenerator { // We don't bother canonicalizing 'cond' here because we only // compare it to zero, and that's invariant under // canonicalization. - let (v1, v2) = if i1.has_pending_f32_nan() != i1.has_pending_f32_nan() + + // If the pending bits of v1 and v2 are the same, we can pass + // them along to the result. Otherwise, apply pending + // canonicalizations now. + let (v1, v2) = if i1.has_pending_f32_nan() != i2.has_pending_f32_nan() || i1.has_pending_f64_nan() != i2.has_pending_f64_nan() { ( diff --git a/lib/llvm-backend/src/state.rs b/lib/llvm-backend/src/state.rs index 357d83d1f00..b13b3c358ec 100644 --- a/lib/llvm-backend/src/state.rs +++ b/lib/llvm-backend/src/state.rs @@ -68,7 +68,7 @@ impl ControlFrame { } } -#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)] +#[derive(Debug, Default, Eq, PartialEq, Copy, Clone, Hash)] pub struct ExtraInfo { state: u8, } @@ -76,61 +76,65 @@ impl ExtraInfo { // This value is required to be arithmetic 32-bit NaN (or 32x4) by the WAsm // machine, but which might not be in the LLVM value. The conversion to // arithmetic NaN is pending. It is required for correctness. - pub fn pending_f32_nan() -> ExtraInfo { + // + // When applied to a 64-bit value, this flag has no meaning and must be + // ignored. It may be set in such cases to allow for common handling of + // 32 and 64-bit operations. + pub const fn pending_f32_nan() -> ExtraInfo { ExtraInfo { state: 1 } } // This value is required to be arithmetic 64-bit NaN (or 64x2) by the WAsm // machine, but which might not be in the LLVM value. The conversion to // arithmetic NaN is pending. It is required for correctness. - pub fn pending_f64_nan() -> ExtraInfo { + // + // When applied to a 32-bit value, this flag has no meaning and must be + // ignored. It may be set in such cases to allow for common handling of + // 32 and 64-bit operations. + pub const fn pending_f64_nan() -> ExtraInfo { ExtraInfo { state: 2 } } // This value either does not contain a 32-bit NaN, or it contains an // arithmetic NaN. In SIMD, applies to all 4 lanes. - pub fn arithmetic_f32() -> ExtraInfo { + pub const fn arithmetic_f32() -> ExtraInfo { ExtraInfo { state: 4 } } // This value either does not contain a 64-bit NaN, or it contains an // arithmetic NaN. In SIMD, applies to both lanes. - pub fn arithmetic_f64() -> ExtraInfo { + pub const fn arithmetic_f64() -> ExtraInfo { ExtraInfo { state: 8 } } - pub fn has_pending_f32_nan(&self) -> bool { + pub const fn has_pending_f32_nan(&self) -> bool { self.state & ExtraInfo::pending_f32_nan().state != 0 } - pub fn has_pending_f64_nan(&self) -> bool { + pub const fn has_pending_f64_nan(&self) -> bool { self.state & ExtraInfo::pending_f64_nan().state != 0 } - pub fn is_arithmetic_f32(&self) -> bool { + pub const fn is_arithmetic_f32(&self) -> bool { self.state & ExtraInfo::arithmetic_f32().state != 0 } - pub fn is_arithmetic_f64(&self) -> bool { + pub const fn is_arithmetic_f64(&self) -> bool { self.state & ExtraInfo::arithmetic_f64().state != 0 } - pub fn strip_pending(&self) -> ExtraInfo { + pub const fn strip_pending(&self) -> ExtraInfo { ExtraInfo { state: self.state & !(ExtraInfo::pending_f32_nan().state | ExtraInfo::pending_f64_nan().state), } } } -impl Default for ExtraInfo { - fn default() -> Self { - ExtraInfo { state: 0 } - } -} + // Union two ExtraInfos. impl BitOr for ExtraInfo { type Output = Self; fn bitor(self, other: Self) -> Self { - assert!(!(self.has_pending_f32_nan() && other.has_pending_f64_nan())); - assert!(!(self.has_pending_f64_nan() && other.has_pending_f32_nan())); + debug_assert!(!(self.has_pending_f32_nan() && other.has_pending_f64_nan())); + debug_assert!(!(self.has_pending_f64_nan() && other.has_pending_f32_nan())); ExtraInfo { state: if self.is_arithmetic_f32() || other.is_arithmetic_f32() { ExtraInfo::arithmetic_f32().state