From 6f1f1f8141a13174779bef381d3bd92bbb3ad994 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Tue, 17 Jan 2023 16:22:27 -0800 Subject: [PATCH 1/4] Turn on egraph-based optimization by default. Also update the pass-driving logic to run egraph optimization only when optimization is enabled. Previously `use_egraphs` acted as an overall enable-switch that drove egraph-based optimization, regardless of `opt_level`. --- cranelift/codegen/meta/src/shared/settings.rs | 8 +++++--- cranelift/codegen/src/context.rs | 16 +++++++++------- cranelift/codegen/src/machinst/isle.rs | 6 ++++-- cranelift/codegen/src/settings.rs | 2 +- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/cranelift/codegen/meta/src/shared/settings.rs b/cranelift/codegen/meta/src/shared/settings.rs index e51cd5a5ac56..2db1f1303439 100644 --- a/cranelift/codegen/meta/src/shared/settings.rs +++ b/cranelift/codegen/meta/src/shared/settings.rs @@ -58,10 +58,12 @@ pub(crate) fn define() -> SettingGroup { "Enable egraph-based optimization.", r#" This enables an optimization phase that converts CLIF to an egraph (equivalence graph) - representation, performs various rewrites, and then converts it back. This can result in - better optimization, but is currently considered experimental. + representation, performs various rewrites, and then converts it back. This should result in + better optimization, but the traditional optimization pass structure is also still + available by setting this to `false`. The `false` setting will eventually be + deprecated and removed. "#, - false, + true, ); settings.add_bool( diff --git a/cranelift/codegen/src/context.rs b/cranelift/codegen/src/context.rs index 175f8e8ea082..4da6cbe04a8a 100644 --- a/cranelift/codegen/src/context.rs +++ b/cranelift/codegen/src/context.rs @@ -185,18 +185,20 @@ impl Context { self.compute_domtree(); self.eliminate_unreachable_code(isa)?; - if isa.flags().use_egraphs() || opt_level != OptLevel::None { + if opt_level != OptLevel::None { self.dce(isa)?; } self.remove_constant_phis(isa)?; - if isa.flags().use_egraphs() { - self.egraph_pass()?; - } else if opt_level != OptLevel::None && isa.flags().enable_alias_analysis() { - for _ in 0..2 { - self.replace_redundant_loads()?; - self.simple_gvn(isa)?; + if opt_level != OptLevel::None { + if isa.flags().use_egraphs() { + self.egraph_pass()?; + } else if isa.flags().enable_alias_analysis() { + for _ in 0..2 { + self.replace_redundant_loads()?; + self.simple_gvn(isa)?; + } } } diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs index def41ade395d..30fcd7c24bc4 100644 --- a/cranelift/codegen/src/machinst/isle.rs +++ b/cranelift/codegen/src/machinst/isle.rs @@ -16,7 +16,7 @@ pub use crate::machinst::{ ABIArg, ABIArgSlot, InputSourceInst, Lower, LowerBackend, RealReg, Reg, RelocDistance, Sig, VCodeInst, Writable, }; -pub use crate::settings::TlsModel; +pub use crate::settings::{OptLevel, TlsModel}; pub type Unit = (); pub type ValueSlice = (ValueList, usize); @@ -131,7 +131,9 @@ macro_rules! isle_lower_prelude_methods { // use. This lowers register pressure. (Only do this if we are // not using egraph-based compilation; the egraph framework // more efficiently rematerializes constants where needed.) - if !self.backend.flags().use_egraphs() { + if !(self.backend.flags().use_egraphs() + && self.backend.flags().opt_level() != OptLevel::None) + { let inputs = self.lower_ctx.get_value_as_source_or_const(val); if inputs.constant.is_some() { let insn = match inputs.inst { diff --git a/cranelift/codegen/src/settings.rs b/cranelift/codegen/src/settings.rs index 27be5c17f65b..6836646e2a8d 100644 --- a/cranelift/codegen/src/settings.rs +++ b/cranelift/codegen/src/settings.rs @@ -528,7 +528,7 @@ probestack_strategy = "outline" regalloc_checker = false regalloc_verbose_logs = false enable_alias_analysis = true -use_egraphs = false +use_egraphs = true enable_verifier = true is_pic = false use_colocated_libcalls = false From a41aa468dd8e306d596e8ebef0afe2d0ba6a5392 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Tue, 17 Jan 2023 16:48:31 -0800 Subject: [PATCH 2/4] Update egraph filetests to set opt_level explicitly and consistently. --- cranelift/filetests/filetests/egraph/algebraic.clif | 2 +- cranelift/filetests/filetests/egraph/alias_analysis.clif | 2 +- cranelift/filetests/filetests/egraph/basic-gvn.clif | 2 +- cranelift/filetests/filetests/egraph/cprop.clif | 2 +- cranelift/filetests/filetests/egraph/i128-opts.clif | 2 +- cranelift/filetests/filetests/egraph/isplit.clif | 2 +- cranelift/filetests/filetests/egraph/issue-5405.clif | 2 +- cranelift/filetests/filetests/egraph/issue-5437.clif | 1 + cranelift/filetests/filetests/egraph/licm.clif | 2 +- cranelift/filetests/filetests/egraph/misc.clif | 2 +- cranelift/filetests/filetests/egraph/multivalue.clif | 1 + cranelift/filetests/filetests/egraph/not_a_load.clif | 1 + cranelift/filetests/filetests/egraph/remat.clif | 2 +- 13 files changed, 13 insertions(+), 10 deletions(-) diff --git a/cranelift/filetests/filetests/egraph/algebraic.clif b/cranelift/filetests/filetests/egraph/algebraic.clif index 6eaa6fcda966..3a852b0ebbef 100644 --- a/cranelift/filetests/filetests/egraph/algebraic.clif +++ b/cranelift/filetests/filetests/egraph/algebraic.clif @@ -1,5 +1,5 @@ test optimize -set opt_level=none +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/alias_analysis.clif b/cranelift/filetests/filetests/egraph/alias_analysis.clif index ce784314696f..87bc5073638b 100644 --- a/cranelift/filetests/filetests/egraph/alias_analysis.clif +++ b/cranelift/filetests/filetests/egraph/alias_analysis.clif @@ -1,5 +1,5 @@ test optimize -set opt_level=none +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/basic-gvn.clif b/cranelift/filetests/filetests/egraph/basic-gvn.clif index 7b387862281e..823614ea27d4 100644 --- a/cranelift/filetests/filetests/egraph/basic-gvn.clif +++ b/cranelift/filetests/filetests/egraph/basic-gvn.clif @@ -1,5 +1,5 @@ test optimize -set opt_level=none +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/cprop.clif b/cranelift/filetests/filetests/egraph/cprop.clif index bc76e1d546e7..261be6fdf431 100644 --- a/cranelift/filetests/filetests/egraph/cprop.clif +++ b/cranelift/filetests/filetests/egraph/cprop.clif @@ -1,5 +1,5 @@ test optimize -set opt_level=none +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/i128-opts.clif b/cranelift/filetests/filetests/egraph/i128-opts.clif index 24737e2c99ab..f30b80bd25c1 100644 --- a/cranelift/filetests/filetests/egraph/i128-opts.clif +++ b/cranelift/filetests/filetests/egraph/i128-opts.clif @@ -1,5 +1,5 @@ test optimize -set opt_level=speed_and_size +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/isplit.clif b/cranelift/filetests/filetests/egraph/isplit.clif index e5cb3ea49de9..e40c32fef84a 100644 --- a/cranelift/filetests/filetests/egraph/isplit.clif +++ b/cranelift/filetests/filetests/egraph/isplit.clif @@ -1,6 +1,6 @@ test interpret test run -set opt_level=speed_and_size +set opt_level=speed set use_egraphs=true set enable_llvm_abi_extensions=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/issue-5405.clif b/cranelift/filetests/filetests/egraph/issue-5405.clif index c453875de5cf..db6f582ec7bf 100644 --- a/cranelift/filetests/filetests/egraph/issue-5405.clif +++ b/cranelift/filetests/filetests/egraph/issue-5405.clif @@ -1,6 +1,6 @@ test interpret test run -set opt_level=speed_and_size +set opt_level=speed set use_egraphs=true target aarch64 diff --git a/cranelift/filetests/filetests/egraph/issue-5437.clif b/cranelift/filetests/filetests/egraph/issue-5437.clif index b8eb3c938b4f..0f20d2f5a0a1 100644 --- a/cranelift/filetests/filetests/egraph/issue-5437.clif +++ b/cranelift/filetests/filetests/egraph/issue-5437.clif @@ -1,4 +1,5 @@ test compile +set opt_level=speed set use_egraphs=true target x86_64 target aarch64 diff --git a/cranelift/filetests/filetests/egraph/licm.clif b/cranelift/filetests/filetests/egraph/licm.clif index a6f4585567ae..3b8b62514946 100644 --- a/cranelift/filetests/filetests/egraph/licm.clif +++ b/cranelift/filetests/filetests/egraph/licm.clif @@ -1,5 +1,5 @@ test optimize -set opt_level=none +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/misc.clif b/cranelift/filetests/filetests/egraph/misc.clif index 668c643cd5ea..811211601bf7 100644 --- a/cranelift/filetests/filetests/egraph/misc.clif +++ b/cranelift/filetests/filetests/egraph/misc.clif @@ -1,5 +1,5 @@ test optimize -set opt_level=none +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/multivalue.clif b/cranelift/filetests/filetests/egraph/multivalue.clif index f2e2e1147251..261fbc75ae1f 100644 --- a/cranelift/filetests/filetests/egraph/multivalue.clif +++ b/cranelift/filetests/filetests/egraph/multivalue.clif @@ -1,4 +1,5 @@ test compile precise-output +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/not_a_load.clif b/cranelift/filetests/filetests/egraph/not_a_load.clif index fde8c2d0e63d..99eefaccaa40 100644 --- a/cranelift/filetests/filetests/egraph/not_a_load.clif +++ b/cranelift/filetests/filetests/egraph/not_a_load.clif @@ -1,4 +1,5 @@ test compile precise-output +set opt_level=speed set use_egraphs=true target x86_64 diff --git a/cranelift/filetests/filetests/egraph/remat.clif b/cranelift/filetests/filetests/egraph/remat.clif index 69289b7cdf3b..f1ba8dd2064a 100644 --- a/cranelift/filetests/filetests/egraph/remat.clif +++ b/cranelift/filetests/filetests/egraph/remat.clif @@ -1,5 +1,5 @@ test optimize -set opt_level=none +set opt_level=speed set use_egraphs=true target x86_64 From 37f02dbf1262570f2645fae2c739b67b35aa1af3 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Thu, 19 Jan 2023 11:59:25 -0800 Subject: [PATCH 3/4] Make some tests explicitly use non-egraphs path (egraphs are tested separately). --- .../filetests/wasm/duplicate-loads-dynamic-memory.wat | 5 +++-- .../filetests/wasm/duplicate-loads-static-memory.wat | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory.wat b/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory.wat index aab07174d14c..ac5d606cd089 100644 --- a/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory.wat +++ b/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory.wat @@ -4,7 +4,8 @@ ;;! ;;! settings = [ ;;! "enable_heap_access_spectre_mitigation=true", -;;! "opt_level=speed_and_size" +;;! "opt_level=speed_and_size", +;;! "use_egraphs=false" ;;! ] ;;! ;;! [globals.vmctx] @@ -112,4 +113,4 @@ ;; ;; block1: ;; @006e return v14, v14 -;; } \ No newline at end of file +;; } diff --git a/cranelift/filetests/filetests/wasm/duplicate-loads-static-memory.wat b/cranelift/filetests/filetests/wasm/duplicate-loads-static-memory.wat index 6a82e60b10a8..bef294915803 100644 --- a/cranelift/filetests/filetests/wasm/duplicate-loads-static-memory.wat +++ b/cranelift/filetests/filetests/wasm/duplicate-loads-static-memory.wat @@ -4,7 +4,8 @@ ;;! ;;! settings = [ ;;! "enable_heap_access_spectre_mitigation=true", -;;! "opt_level=speed_and_size" +;;! "opt_level=speed_and_size", +;;! "use_egraphs=false" ;;! ] ;;! ;;! [globals.vmctx] From a5acda50c73be688cab2449a7b9d7991c63c8f67 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Thu, 19 Jan 2023 12:27:20 -0800 Subject: [PATCH 4/4] Update Wasmtime configuration documentation for `cranelift_use_egraphs`. --- crates/wasmtime/src/config.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index b3c32f2bc659..7e42afbc40a6 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -832,14 +832,13 @@ impl Config { /// Configures the Cranelift code generator to use its /// "egraph"-based mid-end optimizer. /// - /// This optimizer is intended to replace the compiler's more - /// traditional pipeline of optimization passes with a unified - /// code-rewriting system. It is not yet on by default, but it is - /// intended to become the default in a future version. It may - /// result in faster code, at the cost of slightly more - /// compilation-time work. - /// - /// The default value for this is `false`. + /// This optimizer has replaced the compiler's more traditional + /// pipeline of optimization passes with a unified code-rewriting + /// system. It is on by default, but the traditional optimization + /// pass structure is still available for now (it is deprecrated and + /// will be removed in a future version). + /// + /// The default value for this is `true`. #[cfg(compiler)] #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs pub fn cranelift_use_egraphs(&mut self, enable: bool) -> &mut Self {