Skip to content

Commit

Permalink
Make NaN canonicalization a configurable behavior
Browse files Browse the repository at this point in the history
Compilers now default to no canonicalization. This can be enable using
the dedicated functions, either through the Rust API or C API.
  • Loading branch information
jubianchi committed Mar 3, 2021
1 parent 1ee7b4a commit 34425fd
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 4 deletions.
44 changes: 44 additions & 0 deletions lib/c-api/src/wasm_c_api/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ pub struct wasm_config_t {
engine: wasmer_engine_t,
#[cfg(feature = "compiler")]
compiler: wasmer_compiler_t,
nan_canonicalization: bool,
pub(super) features: Option<Box<wasmer_features_t>>,
pub(super) target: Option<Box<wasmer_target_t>>,
}
Expand Down Expand Up @@ -269,6 +270,45 @@ pub extern "C" fn wasm_config_set_engine(config: &mut wasm_config_t, engine: was
config.engine = engine;
}

/// Updates the configuration to enable NaN canonicalization.
///
/// This is a Wasmer-specific function.
///
/// # Example
///
/// ```rust
/// # use inline_c::assert_c;
/// # fn main() {
/// # (assert_c! {
/// # #include "tests/wasmer_wasm.h"
/// #
/// int main() {
/// // Create the configuration.
/// wasm_config_t* config = wasm_config_new();
///
/// // Enable NaN canonicalization.
/// wasm_config_enable_nan_canonicalization(config);
///
/// // Create the engine.
/// wasm_engine_t* engine = wasm_engine_new_with_config(config);
///
/// // Check we have an engine!
/// assert(engine);
///
/// // Free everything.
/// wasm_engine_delete(engine);
///
/// return 0;
/// }
/// # })
/// # .success();
/// # }
/// ```
#[no_mangle]
pub extern "C" fn wasm_config_enable_nan_canonicalization(config: &mut wasm_config_t) {
config.nan_canonicalization = true;
}

/// An engine is used by the store to drive the compilation and the
/// execution of a WebAssembly module.
///
Expand Down Expand Up @@ -471,6 +511,10 @@ pub extern "C" fn wasm_engine_new_with_config(
},
};

if (config.nan_canonicalization) {
compiler_config.enable_nan_canonicalization();
}

let inner: Arc<dyn Engine + Send + Sync> = match config.engine {
wasmer_engine_t::JIT => {
cfg_if! {
Expand Down
38 changes: 38 additions & 0 deletions lib/c-api/wasmer.h
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,44 @@ typedef struct {
extern "C" {
#endif // __cplusplus

/**
* Updates the configuration to enable NaN canonicalization.
*
* This is a Wasmer-specific function.
*
* # Example
*
* ```rust
* # use inline_c::assert_c;
* # fn main() {
* # (assert_c! {
* # #include "tests/wasmer_wasm.h"
* #
* int main() {
* // Create the configuration.
* wasm_config_t* config = wasm_config_new();
*
* // Enable NaN canonicalization.
* wasm_config_enable_nan_canonicalization(config);
*
* // Create the engine.
* wasm_engine_t* engine = wasm_engine_new_with_config(config);
*
* // Check we have an engine!
* assert(engine);
*
* // Free everything.
* wasm_engine_delete(engine);
*
* return 0;
* }
* # })
* # .success();
* # }
* ```
*/
void wasm_config_enable_nan_canonicalization(wasm_config_t *config);

/**
* Creates a new Module from the given wasm bytes.
*
Expand Down
36 changes: 36 additions & 0 deletions lib/c-api/wasmer.hh
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,42 @@ struct wasmer_wasi_map_dir_entry_t {

extern "C" {

/// Updates the configuration to enable NaN canonicalization.
///
/// This is a Wasmer-specific function.
///
/// # Example
///
/// ```rust
/// # use inline_c::assert_c;
/// # fn main() {
/// # (assert_c! {
/// # #include "tests/wasmer_wasm.h"
/// #
/// int main() {
/// // Create the configuration.
/// wasm_config_t* config = wasm_config_new();
///
/// // Enable NaN canonicalization.
/// wasm_config_enable_nan_canonicalization(config);
///
/// // Create the engine.
/// wasm_engine_t* engine = wasm_engine_new_with_config(config);
///
/// // Check we have an engine!
/// assert(engine);
///
/// // Free everything.
/// wasm_engine_delete(engine);
///
/// return 0;
/// }
/// # })
/// # .success();
/// # }
/// ```
void wasm_config_enable_nan_canonicalization(wasm_config_t *config);

/// Creates a new Module from the given wasm bytes.
///
/// Returns `wasmer_result_t::WASMER_OK` upon success.
Expand Down
2 changes: 2 additions & 0 deletions lib/c-api/wasmer_wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ bool wasi_get_unordered_imports(const wasm_store_t *store,
wasi_version_t wasi_get_wasi_version(const wasm_module_t *module);
#endif

void wasm_config_enable_nan_canonicalization(wasm_config_t *config);

#if defined(WASMER_COMPILER_ENABLED)
void wasm_config_set_compiler(wasm_config_t *config, wasmer_compiler_t compiler);
#endif
Expand Down
7 changes: 7 additions & 0 deletions lib/compiler-cranelift/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,21 @@ impl Cranelift {
}

impl CompilerConfig for Cranelift {
/// Emit code suitable for dlopen.
fn enable_pic(&mut self) {
self.enable_pic = true;
}

/// Whether to verify compiler IR.
fn enable_verifier(&mut self) {
self.enable_verifier = true;
}

/// Whether to canonicalize NaNs.
fn enable_nan_canonicalization(&mut self) {
self.enable_nan_canonicalization = true;
}

/// Transform it into the compiler
fn compiler(self: Box<Self>) -> Box<dyn Compiler> {
Box::new(CraneliftCompiler::new(*self))
Expand Down
5 changes: 5 additions & 0 deletions lib/compiler-llvm/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,11 @@ impl CompilerConfig for LLVM {
self.enable_verifier = true;
}

/// Whether to canonicalize NaNs.
fn enable_nan_canonicalization(&mut self) {
self.enable_nan_canonicalization = true;
}

/// Transform it into the compiler.
fn compiler(self: Box<Self>) -> Box<dyn Compiler> {
Box::new(LLVMCompiler::new(*self))
Expand Down
6 changes: 6 additions & 0 deletions lib/compiler-llvm/src/translator/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ impl FuncTranslator {
wasm_module,
symbol_registry,
abi: &*self.abi,
config,
};
fcg.ctx.add_func(
func_index,
Expand Down Expand Up @@ -859,6 +860,10 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
value: BasicValueEnum<'ctx>,
info: ExtraInfo,
) -> BasicValueEnum<'ctx> {
if !self.config.enable_nan_canonicalization {
return value;
}

if info.has_pending_f32_nan() {
if value.get_type().is_vector_type()
|| value.get_type() == self.intrinsics.i128_ty.as_basic_type_enum()
Expand Down Expand Up @@ -1287,6 +1292,7 @@ pub struct LLVMFunctionCodeGenerator<'ctx, 'a> {
unreachable_depth: usize,
memory_styles: &'a PrimaryMap<MemoryIndex, MemoryStyle>,
_table_styles: &'a PrimaryMap<TableIndex, TableStyle>,
config: &'a LLVM,

// This is support for stackmaps:
/*
Expand Down
7 changes: 3 additions & 4 deletions lib/compiler-singlepass/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl Singlepass {
/// specified.
pub fn new() -> Self {
Self {
enable_nan_canonicalization: true,
enable_nan_canonicalization: false,
enable_stack_check: false,
middlewares: vec![],
}
Expand Down Expand Up @@ -48,9 +48,8 @@ impl Singlepass {
}

impl CompilerConfig for Singlepass {
fn enable_pic(&mut self) {
// Do nothing, since singlepass already emits
// PIC code.
fn enable_nan_canonicalization(&mut self) {
self.enable_nan_canonicalization = true;
}

/// Transform it into the compiler
Expand Down
6 changes: 6 additions & 0 deletions lib/compiler/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ pub trait CompilerConfig {
// in case they create an IR that they can verify.
}

/// Enable NaN canonicalization.
fn enable_nan_canonicalization(&mut self) {
// By default we do nothing, each backend will need to customize this
// in case they create an IR that they can verify.
}

/// Gets the custom compiler config
fn compiler(self: Box<Self>) -> Box<dyn Compiler>;

Expand Down
3 changes: 3 additions & 0 deletions tests/ignores.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ cranelift::spec::simd::simd_i8x16_sat_arith on aarch64
cranelift::spec::simd::simd_lane on aarch64
cranelift::spec::skip_stack_guard_page on aarch64

# TODO(https://github.com/wasmerio/wasmer/issues/1727): Need to review NaN canonicalization
wast::llvm::spec::float_exprs
wast::llvm::spec::simd::simd_f64x2_arith

# SIMD changes
# due to breaking changes in the SIMD proposal, we have to disable these spec tests
Expand Down

0 comments on commit 34425fd

Please sign in to comment.