From 1faff8c2cea58e8d499c54cd99c06e5e90e64523 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Thu, 19 Jan 2023 15:46:53 -0800 Subject: [PATCH] Enable egraph-based optimization by default. (#5587) This PR follows up on #5382 and #5391, which rebuilt the egraph-based optimization framework to be more performant, by enabling it by default. Based on performance results in #5382 (my measurements on SpiderMonkey and bjorn3's independent confirmation with cg_clif), it seems that this is reasonable to enable. Now that we have been fuzzing compiler configurations with egraph opts (#5388) for 6 weeks, having fixed a few fuzzbugs that came up (#5409, #5420, #5438) and subsequently received no further reports from OSS-Fuzz, I believe it is stable enough to rely on. This PR enables `use_egraphs`, and also normalizes its meaning: previously it forced optimization (it basically meant "turn on the egraph optimization machinery"), now it runs egraph opts if the opt level indicates (it means "use egraphs to optimize if we are going to optimize"). The conditionals in the top-level pass driver are a little subtle, but will get simpler once we can remove the non-egraph path (which we plan to do eventually!). Fixes #5181. --- 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 +- .../filetests/filetests/egraph/algebraic.clif | 2 +- .../filetests/egraph/alias_analysis.clif | 2 +- .../filetests/filetests/egraph/basic-gvn.clif | 2 +- cranelift/filetests/filetests/egraph/cprop.clif | 2 +- .../filetests/filetests/egraph/i128-opts.clif | 2 +- cranelift/filetests/filetests/egraph/isplit.clif | 2 +- .../filetests/filetests/egraph/issue-5405.clif | 2 +- .../filetests/filetests/egraph/issue-5437.clif | 1 + cranelift/filetests/filetests/egraph/licm.clif | 2 +- cranelift/filetests/filetests/egraph/misc.clif | 2 +- .../filetests/filetests/egraph/multivalue.clif | 1 + .../filetests/filetests/egraph/not_a_load.clif | 1 + cranelift/filetests/filetests/egraph/remat.clif | 2 +- .../wasm/duplicate-loads-dynamic-memory.wat | 5 +++-- .../wasm/duplicate-loads-static-memory.wat | 3 ++- crates/wasmtime/src/config.rs | 13 ++++++------- 20 files changed, 43 insertions(+), 33 deletions(-) diff --git a/cranelift/codegen/meta/src/shared/settings.rs b/cranelift/codegen/meta/src/shared/settings.rs index e51cd5a5ac..2db1f13034 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 175f8e8ea0..4da6cbe04a 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 def41ade39..30fcd7c24b 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 27be5c17f6..6836646e2a 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 diff --git a/cranelift/filetests/filetests/egraph/algebraic.clif b/cranelift/filetests/filetests/egraph/algebraic.clif index 6eaa6fcda9..3a852b0ebb 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 ce78431469..87bc507363 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 7b38786228..823614ea27 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 bc76e1d546..261be6fdf4 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 24737e2c99..f30b80bd25 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 e5cb3ea49d..e40c32fef8 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 c453875de5..db6f582ec7 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 b8eb3c938b..0f20d2f5a0 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 a6f4585567..3b8b625149 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 668c643cd5..811211601b 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 f2e2e11472..261fbc75ae 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 fde8c2d0e6..99eefaccaa 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 69289b7cdf..f1ba8dd206 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 diff --git a/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory.wat b/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory.wat index aab07174d1..ac5d606cd0 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 6a82e60b10..bef2949158 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] diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index b3c32f2bc6..7e42afbc40 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. + /// 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 `false`. + /// 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 {