From 0af8737ec36ce73ac9f88510fe8d1ec39ed1d4fc Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 18 Apr 2022 14:06:07 -0700 Subject: [PATCH] Add support for running the regalloc2 checker. (#4043) With these fixes, all this PR has to do is instantiate and run the checker on the `regalloc2::Output`. This is off by default, and is enabled by setting the `regalloc_checker` Cranelift option. This restores the old functionality provided by e.g. the `backtracking_checked` regalloc algorithm setting rather than `backtracking` when we were still on regalloc.rs. --- Cargo.lock | 4 ++-- cranelift/codegen/Cargo.toml | 2 +- cranelift/codegen/meta/src/shared/settings.rs | 15 +++++++++++++++ cranelift/codegen/src/machinst/compile.rs | 18 ++++++++++++++++++ cranelift/codegen/src/settings.rs | 1 + cranelift/codegen/src/timing.rs | 1 + crates/wasmtime/src/engine.rs | 1 + 7 files changed, 39 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1084250a02..39d3f1188d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2316,9 +2316,9 @@ dependencies = [ [[package]] name = "regalloc2" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3dd122b168f0046afcde717e002cdf76c9c87f829ae99dd12a02a0dcf7cc68f1" +checksum = "1ec1c2e4f4b879982653d243d9561d9f49004ce15a47e9682ad1f6d53a06aac0" dependencies = [ "fxhash", "log", diff --git a/cranelift/codegen/Cargo.toml b/cranelift/codegen/Cargo.toml index 51f39a3674..cb8bb69be7 100644 --- a/cranelift/codegen/Cargo.toml +++ b/cranelift/codegen/Cargo.toml @@ -23,7 +23,7 @@ serde = { version = "1.0.94", features = ["derive"], optional = true } bincode = { version = "1.2.1", optional = true } gimli = { version = "0.26.0", default-features = false, features = ["write"], optional = true } smallvec = { version = "1.6.1" } -regalloc2 = { version = "0.1.1", features = ["checker"] } +regalloc2 = { version = "0.1.2", features = ["checker"] } souper-ir = { version = "2.1.0", optional = true } # It is a goal of the cranelift-codegen crate to have minimal external dependencies. # Please don't add any unless they are essential to the task of creating binary diff --git a/cranelift/codegen/meta/src/shared/settings.rs b/cranelift/codegen/meta/src/shared/settings.rs index 58b7cd4499..aa829b195c 100644 --- a/cranelift/codegen/meta/src/shared/settings.rs +++ b/cranelift/codegen/meta/src/shared/settings.rs @@ -3,6 +3,21 @@ use crate::cdsl::settings::{SettingGroup, SettingGroupBuilder}; pub(crate) fn define() -> SettingGroup { let mut settings = SettingGroupBuilder::new("shared"); + settings.add_bool( + "regalloc_checker", + "Enable the symbolic checker for register allocation.", + r#" + This performs a verification that the register allocator preserves + equivalent dataflow with respect to the original (pre-regalloc) + program. This analysis is somewhat expensive. However, if it succeeds, + it provides independent evidence (by a carefully-reviewed, from-first-principles + analysis) that no regalloc bugs were triggered for the particular compilations + performed. This is a valuable assurance to have as regalloc bugs can be + very dangerous and difficult to debug. + "#, + false, + ); + settings.add_enum( "opt_level", "Optimization level for generated code.", diff --git a/cranelift/codegen/src/machinst/compile.rs b/cranelift/codegen/src/machinst/compile.rs index 1bdfe24b52..55b5965ecd 100644 --- a/cranelift/codegen/src/machinst/compile.rs +++ b/cranelift/codegen/src/machinst/compile.rs @@ -47,5 +47,23 @@ pub fn compile( .expect("register allocation") }; + // Run the regalloc checker, if requested. + if b.flags().regalloc_checker() { + let _tt = timing::regalloc_checker(); + let mut checker = regalloc2::checker::Checker::new(&vcode, machine_env); + checker.prepare(®alloc_result); + checker + .run() + .map_err(|err| { + log::error!( + "Register allocation checker errors:\n{:?}\nfor vcode:\n{:?}", + err, + vcode + ); + err + }) + .expect("register allocation checker"); + } + Ok((vcode, regalloc_result)) } diff --git a/cranelift/codegen/src/settings.rs b/cranelift/codegen/src/settings.rs index 5a9f27c746..da8e9a1fa3 100644 --- a/cranelift/codegen/src/settings.rs +++ b/cranelift/codegen/src/settings.rs @@ -515,6 +515,7 @@ tls_model = "none" libcall_call_conv = "isa_default" baldrdash_prologue_words = 0 probestack_size_log2 = 12 +regalloc_checker = false enable_verifier = true is_pic = false use_colocated_libcalls = false diff --git a/cranelift/codegen/src/timing.rs b/cranelift/codegen/src/timing.rs index 4d25560e8f..8155241018 100644 --- a/cranelift/codegen/src/timing.rs +++ b/cranelift/codegen/src/timing.rs @@ -65,6 +65,7 @@ define_passes! { vcode_emit_finish: "VCode emission finalization", regalloc: "Register allocation", + regalloc_checker: "Register allocation symbolic verification", binemit: "Binary machine code emission", layout_renumber: "Layout full renumbering", diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index 083905e533..abb7881837 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -336,6 +336,7 @@ impl Engine { | "enable_float" | "enable_simd" | "enable_verifier" + | "regalloc_checker" | "is_pic" | "machine_code_cfg_info" | "tls_model" // wasmtime doesn't use tls right now