Merge #1332

1332: Add `CompilerConfig` opt to disable IR verification in debug mode r=MarkMcCaskey a=MarkMcCaskey

Resolves #1330 

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <mark@wasmer.io>

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## **[Unreleased]**
 
+- [#1332](https://github.com/wasmerio/wasmer/pull/1332) Add option to `CompilerConfig` to force compiler IR verification off even when `debug_assertions` are enabled. This can be used to make debug builds faster, which may be important if you're creating a library that wraps Wasmer and depend on the speed of debug builds.
 - [#1320](https://github.com/wasmerio/wasmer/pull/1320) Change `custom_sections` field in `ModuleInfo` to be more standards compliant by allowing multiple custom sections with the same name. To get the old behavior with the new API, you can add `.last().unwrap()` to accesses. For example, `module_info.custom_sections["custom_section_name"].last().unwrap()`.
 - [#1303](https://github.com/wasmerio/wasmer/pull/1303) NaN canonicalization for singlepass backend.
 - [#1292](https://github.com/wasmerio/wasmer/pull/1292) Experimental Support for Android (x86_64 and AArch64)

lib/clif-backend/src/lib.rs

@@ -43,18 +43,27 @@ fn get_isa(config: Option<&CompilerConfig>) -> Box<dyn isa::TargetIsa> {
         builder.set("opt_level", "speed_and_size").unwrap();
         builder.set("enable_jump_tables", "false").unwrap();
 
-        if cfg!(test) || cfg!(debug_assertions) {
-            builder.set("enable_verifier", "true").unwrap();
-        } else {
-            builder.set("enable_verifier", "false").unwrap();
-        }
+        let enable_verifier: bool;
 
         if let Some(config) = config {
             if config.nan_canonicalization {
                 builder.set("enable_nan_canonicalization", "true").unwrap();
             }
+            enable_verifier = config.enable_verification;
+        } else {
+            // Set defaults if no config found.
+            // NOTE: cfg(test) probably does nothing when not running `cargo test`
+            //       on this crate
+            enable_verifier = cfg!(test) || cfg!(debug_assertions);
         }
 
+        builder
+            .set(
+                "enable_verifier",
+                if enable_verifier { "true" } else { "false" },
+            )
+            .unwrap();
+
         let flags = settings::Flags::new(builder);
         debug_assert_eq!(flags.opt_level(), settings::OptLevel::SpeedAndSize);
         flags

lib/clif-backend/src/trampoline.rs

@@ -86,7 +86,7 @@ impl Trampolines {
             let sig_index = module.func_assoc[*exported_func_index];
             let func_sig = &module.signatures[sig_index];
 
-            let trampoline_func = generate_func(&func_sig);
+            let trampoline_func = generate_func(isa, &func_sig);
 
             ctx.func = trampoline_func;
 
@@ -150,13 +150,13 @@ impl Trampolines {
 
 /// This function generates a trampoline for the specific signature
 /// passed into it.
-fn generate_func(func_sig: &FuncSig) -> ir::Function {
-    let trampoline_sig = generate_trampoline_signature();
+fn generate_func(isa: &dyn isa::TargetIsa, func_sig: &FuncSig) -> ir::Function {
+    let trampoline_sig = generate_trampoline_signature(isa);
 
     let mut func =
         ir::Function::with_name_signature(ir::ExternalName::testcase("trampln"), trampoline_sig);
 
-    let export_sig_ref = func.import_signature(generate_export_signature(func_sig));
+    let export_sig_ref = func.import_signature(generate_export_signature(isa, func_sig));
 
     let entry_ebb = func.dfg.make_block();
     let vmctx_ptr = func.dfg.append_block_param(entry_ebb, ir::types::I64);
@@ -211,8 +211,8 @@ fn wasm_ty_to_clif(ty: Type) -> ir::types::Type {
     }
 }
 
-fn generate_trampoline_signature() -> ir::Signature {
-    let call_convention = super::get_isa(None).default_call_conv();
+fn generate_trampoline_signature(isa: &dyn isa::TargetIsa) -> ir::Signature {
+    let call_convention = isa.default_call_conv();
     let mut sig = ir::Signature::new(call_convention);
 
     let ptr_param = ir::AbiParam {
@@ -227,8 +227,8 @@ fn generate_trampoline_signature() -> ir::Signature {
     sig
 }
 
-fn generate_export_signature(func_sig: &FuncSig) -> ir::Signature {
-    let call_convention = super::get_isa(None).default_call_conv();
+fn generate_export_signature(isa: &dyn isa::TargetIsa, func_sig: &FuncSig) -> ir::Signature {
+    let call_convention = isa.default_call_conv();
     let mut export_clif_sig = ir::Signature::new(call_convention);
 
     let func_sig_iter = func_sig.params().iter().map(|wasm_ty| ir::AbiParam {

lib/runtime-core/src/backend.rs

@@ -106,7 +106,7 @@ impl BackendCompilerConfig {
 }
 
 /// Configuration data for the compiler
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub struct CompilerConfig {
     /// Symbol information generated from emscripten; used for more detailed debug messages
     pub symbol_map: Option<HashMap<u32, String>>,
@@ -136,6 +136,13 @@ pub struct CompilerConfig {
     /// Enabling this makes execution deterministic but increases runtime overhead.
     pub nan_canonicalization: bool,
 
+    /// Turns on verification that is done by default when `debug_assertions` are enabled
+    /// (for example in 'debug' builds). Disabling this flag will make compilation faster
+    /// in debug mode at the cost of not detecting bugs in the compiler.
+    ///
+    /// These verifications are disabled by default in 'release' builds.
+    pub enable_verification: bool,
+
     pub features: Features,
 
     // Target info. Presently only supported by LLVM.
@@ -148,6 +155,30 @@ pub struct CompilerConfig {
     pub generate_debug_info: bool,
 }
 
+impl Default for CompilerConfig {
+    fn default() -> Self {
+        Self {
+            symbol_map: Default::default(),
+            memory_bound_check_mode: Default::default(),
+            enforce_stack_check: Default::default(),
+            track_state: Default::default(),
+            full_preemption: Default::default(),
+            nan_canonicalization: Default::default(),
+            features: Default::default(),
+            triple: Default::default(),
+            cpu_name: Default::default(),
+            cpu_features: Default::default(),
+            backend_specific_config: Default::default(),
+            generate_debug_info: Default::default(),
+
+            // Default verification to 'on' when testing or running in debug mode.
+            // NOTE: cfg(test) probably does nothing when not running `cargo test`
+            //       on this crate
+            enable_verification: cfg!(test) || cfg!(debug_assertions),
+        }
+    }
+}
+
 impl CompilerConfig {
     /// Use this to check if we should be generating debug information.
     /// This function takes into account the features that runtime-core was

lib/spectests/tests/spectest.rs

@@ -337,6 +337,7 @@ mod tests {
                                 threads: true,
                             },
                             nan_canonicalization: true,
+                            enable_verification: true,
                             ..Default::default()
                         };
                         let module = compile_with_config(&module.into_vec(), config)
@@ -776,6 +777,7 @@ mod tests {
                                 threads: true,
                             },
                             nan_canonicalization: true,
+                            enable_verification: true,
                             ..Default::default()
                         };
                         compile_with_config(&module.into_vec(), config)
@@ -829,6 +831,7 @@ mod tests {
                                 threads: true,
                             },
                             nan_canonicalization: true,
+                            enable_verification: true,
                             ..Default::default()
                         };
                         compile_with_config(&module.into_vec(), config)
@@ -881,6 +884,7 @@ mod tests {
                             threads: true,
                         },
                         nan_canonicalization: true,
+                        enable_verification: true,
                         ..Default::default()
                     };
                     let module = compile_with_config(&module.into_vec(), config)
@@ -977,6 +981,7 @@ mod tests {
                                 threads: true,
                             },
                             nan_canonicalization: true,
+                            enable_verification: true,
                             ..Default::default()
                         };
                         let module = compile_with_config(&module.into_vec(), config)