diff --git a/sway-core/src/build_config.rs b/sway-core/src/build_config.rs index a9d7daee0c0..6714c4507f4 100644 --- a/sway-core/src/build_config.rs +++ b/sway-core/src/build_config.rs @@ -6,7 +6,7 @@ use std::{ sync::Arc, }; use strum::{Display, EnumString}; -use sway_ir::{PassManager, PrintPassesOpts, VerifyPassesOpts}; +use sway_ir::{Options, PassManager, VerifyPassesOpts}; #[derive( Clone, @@ -187,14 +187,15 @@ impl std::ops::BitOrAssign for IrCli { } } -impl From<&IrCli> for PrintPassesOpts { +impl From<&IrCli> for Options { fn from(value: &IrCli) -> Self { Self { - initial: value.initial, - r#final: value.r#final, - modified_only: value.modified_only, - metadata: value.print_metadata, - passes: HashSet::from_iter(value.passes.iter().cloned()), + print_initial: value.initial, + print_final: value.r#final, + print_modified_only: value.modified_only, + print_metadata: value.print_metadata, + print_passes: HashSet::from_iter(value.passes.iter().cloned()), + ..Default::default() } } } diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs index fec21d2ca0a..502eb53876a 100644 --- a/sway-core/src/lib.rs +++ b/sway-core/src/lib.rs @@ -64,8 +64,8 @@ use sway_error::handler::{ErrorEmitted, Handler}; use sway_error::warning::{CollectedTraitImpl, CompileInfo, CompileWarning, Info, Warning}; use sway_features::ExperimentalFeatures; use sway_ir::{ - create_o1_pass_group, register_known_passes, Context, Kind, Module, PassGroup, PassManager, - PrintPassesOpts, ARG_DEMOTION_NAME, ARG_POINTEE_MUTABILITY_TAGGER_NAME, CONST_DEMOTION_NAME, + create_o1_pass_group, register_known_passes, Context, Kind, Module, Options, PassGroup, + PassManager, ARG_DEMOTION_NAME, ARG_POINTEE_MUTABILITY_TAGGER_NAME, CONST_DEMOTION_NAME, DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_INLINE_NAME, GLOBALS_DCE_NAME, INIT_AGGR_LOWERING_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MEMCPYPROP_REVERSE_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME, @@ -1614,10 +1614,15 @@ pub(crate) fn compile_ast_to_ir_to_asm( } // Run the passes. - let print_passes_opts: PrintPassesOpts = (&build_config.print_ir).into(); - let res = if let Err(ir_error) = - pass_mgr.run_with_print_verify(&mut ir, &pass_group, &print_passes_opts) - { + let mut options: Options = (&build_config.print_ir).into(); + + let force_verify_ir = std::env::var("SWAY_FORCE_VERIFY_IR") + .map(|v| v.parse::().unwrap_or(false)) + .unwrap_or(false); + options.force_verify_ir = force_verify_ir; + ir.verify_ssa_dominance = force_verify_ir; + + let res = if let Err(ir_error) = pass_mgr.run(&mut ir, &pass_group, &options) { Err(handler.emit_err(CompileError::InternalOwned( ir_error.to_string(), span::Span::dummy(), diff --git a/sway-ir/src/analysis/dominator.rs b/sway-ir/src/analysis/dominator.rs index 4d65b53f4d3..d7f416524b9 100644 --- a/sway-ir/src/analysis/dominator.rs +++ b/sway-ir/src/analysis/dominator.rs @@ -111,13 +111,8 @@ pub fn create_dominators_pass() -> Pass { } } -/// Compute the dominator tree for the CFG. -fn compute_dom_tree( - context: &Context, - analyses: &AnalysisResults, - function: Function, -) -> Result { - let po: &PostOrder = analyses.get_analysis_result(function); +/// Compute the dominator tree of the function, given a post-order traversal. +pub fn compute_dom_tree_from_po(context: &Context, function: Function, po: &PostOrder) -> DomTree { let mut dom_tree = DomTree::default(); let entry = function.get_entry_block(context); @@ -198,7 +193,17 @@ fn compute_dom_tree( dom_tree.0.get_mut(&parent).unwrap().children.push(child); } - Ok(Box::new(dom_tree)) + dom_tree +} + +/// Returns the dominator tree `AnalysisResult` for the function. +fn compute_dom_tree( + context: &Context, + analyses: &AnalysisResults, + function: Function, +) -> Result { + let po: &PostOrder = analyses.get_analysis_result(function); + Ok(Box::new(compute_dom_tree_from_po(context, function, po))) } impl DomTree { diff --git a/sway-ir/src/bin/opt.rs b/sway-ir/src/bin/opt.rs index ef2b69b45c5..a423017e51c 100644 --- a/sway-ir/src/bin/opt.rs +++ b/sway-ir/src/bin/opt.rs @@ -6,7 +6,7 @@ use std::{ use anyhow::anyhow; use sway_features::ExperimentalFeatures; use sway_ir::{ - insert_after_each, register_known_passes, Backtrace, PassGroup, PassManager, + insert_after_each, register_known_passes, Backtrace, Options, PassGroup, PassManager, MODULE_PRINTER_NAME, MODULE_VERIFIER_NAME, }; use sway_types::SourceEngine; @@ -45,7 +45,7 @@ fn main() -> Result<(), anyhow::Error> { if config.verify_after_each { passes = insert_after_each(passes, MODULE_VERIFIER_NAME); } - pass_mgr.run(&mut ir, &passes)?; + pass_mgr.run(&mut ir, &passes, &Options::default())?; // Write the output file or standard out. write_to_output(ir, &config.output_path)?; diff --git a/sway-ir/src/context.rs b/sway-ir/src/context.rs index 9b457a44087..9ba9234e658 100644 --- a/sway-ir/src/context.rs +++ b/sway-ir/src/context.rs @@ -64,6 +64,10 @@ pub struct Context<'eng> { next_unique_sym_tag: u64, next_unique_panic_error_code: u64, next_unique_panicking_call_id: u64, + + /// When enabled, `Context::verify` performs an SSA-dominance legality check: + /// every used value must be defined by a block/instruction that dominates its use. + pub verify_ssa_dominance: bool, } impl<'eng> Context<'eng> { @@ -95,6 +99,8 @@ impl<'eng> Context<'eng> { program_kind: Kind::Contract, experimental, backtrace, + // false by default for performance reasons + verify_ssa_dominance: false, }; Type::create_basic_types(&mut def); def diff --git a/sway-ir/src/error.rs b/sway-ir/src/error.rs index 11d3083cd47..b7e1ef226ed 100644 --- a/sway-ir/src/error.rs +++ b/sway-ir/src/error.rs @@ -99,6 +99,10 @@ pub enum IrError { VerifyInitAggrUnknownInitializerType(usize), VerifyInitAggrMismatchedStructFieldType(usize, String, String), VerifyInitAggrMismatchedArrayElementType(usize, String, String), + VerifyInvalidScope { + value: String, + val: Value, + }, } impl IrError { @@ -107,6 +111,7 @@ impl IrError { Self::VerifyGepFromNonPointer(_, v) => v.as_ref(), Self::VerifyGepInconsistentTypes(_, v) => v.as_ref(), Self::VerifyStoreMismatchedTypes(v) => v.as_ref(), + Self::VerifyInvalidScope { val, .. } => Some(val), _ => None, } } @@ -611,6 +616,12 @@ impl fmt::Display for IrError { "Verification failed: init_aggr instruction has an initializer with a type mismatch for array element at index {idx}. Expected element type: {element_ty}, found initializer type: {initializer_ty}." ) } + IrError::VerifyInvalidScope { value, .. } => { + write!( + f, + "Verification failed: unknown value: {value}", + ) + }, } } } diff --git a/sway-ir/src/optimize.rs b/sway-ir/src/optimize.rs index 72c2136a04d..99604a6b437 100644 --- a/sway-ir/src/optimize.rs +++ b/sway-ir/src/optimize.rs @@ -50,7 +50,7 @@ mod target_fuel; #[cfg(test)] pub mod tests { - use crate::{Backtrace, PassGroup, PassManager}; + use crate::{Backtrace, Options, PassGroup, PassManager}; use sway_features::ExperimentalFeatures; use sway_types::SourceEngine; @@ -104,7 +104,16 @@ pub mod tests { } let before = context.to_string(); - let modified = pass_manager.run(&mut context, &group).unwrap(); + let modified = pass_manager + .run( + &mut context, + &group, + &Options { + rounds: 1, + ..Default::default() + }, + ) + .unwrap(); let after = context.to_string(); // print diff to help debug diff --git a/sway-ir/src/optimize/sroa.rs b/sway-ir/src/optimize/sroa.rs index 840c9aed98b..996655cdefe 100644 --- a/sway-ir/src/optimize/sroa.rs +++ b/sway-ir/src/optimize/sroa.rs @@ -128,10 +128,9 @@ pub fn sroa( }) .collect(); - let mut scalar_replacements = FxHashMap::::default(); - for block in function.block_iter(context) { let mut new_insts = Vec::new(); + let mut replacements_per_block = FxHashMap::::default(); for inst in block.instruction_iter(context) { if let InstOp::MemCopyVal { dst_val_ptr, @@ -372,6 +371,11 @@ pub fn sroa( .next() .filter(|sym| syms.len() == 1 && candidates.contains(sym)) { + // Reuse the `get_local` already in the same block + if replacements_per_block.contains_key(ptr) { + continue; + } + let Some(offset) = combine_indices(context, *ptr).and_then(|indices| { sym.get_type(context) .get_pointee_type(context) @@ -389,15 +393,17 @@ pub fn sroa( let scalarized_local = Value::new_instruction(context, block, InstOp::GetLocal(*remapped_var)); new_insts.push(scalarized_local); - scalar_replacements.insert(*ptr, scalarized_local); + replacements_per_block.insert(*ptr, scalarized_local); } } new_insts.push(inst); } block.take_body(context, new_insts); - } - function.replace_values(context, &scalar_replacements, None); + if !replacements_per_block.is_empty() { + block.replace_values(context, &replacements_per_block); + } + } Ok(true) } diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index 2177c9ee542..90d4d1a13dc 100644 --- a/sway-ir/src/pass_manager.rs +++ b/sway-ir/src/pass_manager.rs @@ -156,19 +156,37 @@ impl AnalysisResults { } } -/// Options for printing [Pass]es in case of running them with printing requested. +/// Options when running the `PassManager`. +/// +/// # Printint Options /// /// Note that states of IR can always be printed by injecting the module printer pass /// and just running the passes. That approach however offers less control over the /// printing. E.g., requiring the printing to happen only if the previous passes /// modified the IR cannot be done by simply injecting a module printer. #[derive(Debug)] -pub struct PrintPassesOpts { - pub initial: bool, - pub r#final: bool, - pub modified_only: bool, - pub metadata: bool, - pub passes: HashSet, +pub struct Options { + pub print_initial: bool, + pub print_final: bool, + pub print_modified_only: bool, + pub print_metadata: bool, + pub print_passes: HashSet, + pub force_verify_ir: bool, + pub rounds: usize, +} + +impl Default for Options { + fn default() -> Self { + Self { + print_initial: false, + print_final: false, + print_modified_only: false, + print_metadata: false, + print_passes: HashSet::default(), + force_verify_ir: false, + rounds: 2, + } + } } /// Options for verifying [Pass]es in case of running them with verifying requested. @@ -363,25 +381,16 @@ impl PassManager { Ok(modified) } - /// Run the `passes` and return true if the `passes` modify the initial `ir`. - pub fn run(&mut self, ir: &mut Context, passes: &PassGroup) -> Result { - let mut modified = false; - for pass in passes.flatten_pass_group() { - modified |= self.actually_run(ir, pass)?; - } - Ok(modified) - } - /// Run the `passes` and return true if the `passes` modify the initial `ir`. /// The IR states are printed according to the options provided and verified. - pub fn run_with_print_verify( + pub fn run( &mut self, ir: &mut Context, passes: &PassGroup, - print_opts: &PrintPassesOpts, + options: &Options, ) -> Result { - if print_opts.initial { - print_initial_or_final_ir(ir, "Initial", print_opts.metadata); + if options.print_initial { + print_initial_or_final_ir(ir, "Initial", options.print_metadata); } // Verify before we start @@ -389,17 +398,12 @@ impl PassManager { let mut global_modified = false; - // Make it easy for tests to run IR verification in all steps - let force_verify: String = - std::env::var("SWAY_FORCE_VERIFY_IR").unwrap_or_else(|_| "false".to_string()); - let force_verify: bool = force_verify.parse().unwrap_or(false); - - for _ in 0..2 { + for _ in 0..options.rounds { let mut iter_modified = false; for pass in passes.flatten_pass_group() { // Save IR before optimisation only when forcing verification - let ir_before = if force_verify { + let ir_before = if options.force_verify_ir { ir.to_string() } else { String::new() @@ -409,7 +413,7 @@ impl PassManager { let modified = self.actually_run(ir, pass)?; // Save IR after optimisation only when forcing verification - let ir_after = if force_verify { + let ir_after = if options.force_verify_ir { ir.to_string() } else { String::new() @@ -417,17 +421,18 @@ impl PassManager { iter_modified |= modified; - if print_opts.passes.contains(pass) && (!print_opts.modified_only || modified) { + if options.print_passes.contains(pass) && (!options.print_modified_only || modified) + { print_ir_after_pass( ir, self.lookup_registered_pass(pass).unwrap(), - print_opts.metadata, + options.print_metadata, ); } ir.verify()?; - if force_verify { + if options.force_verify_ir { // Verify pass correctly return modified let ir_modified = ir_before != ir_after; if modified != ir_modified { @@ -446,8 +451,8 @@ impl PassManager { } } - if print_opts.r#final { - print_initial_or_final_ir(ir, "Final", print_opts.metadata); + if options.print_final { + print_initial_or_final_ir(ir, "Final", options.print_metadata); } Ok(global_modified) diff --git a/sway-ir/src/verify.rs b/sway-ir/src/verify.rs index 2340a778e76..5edf7093d0f 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -3,9 +3,8 @@ //! During creation, deserialization and optimization the IR should be verified to be in a //! consistent valid state, using the functions in this module. -use itertools::Itertools; - use crate::{ + analysis::dominator::{compute_dom_tree_from_po, compute_post_order, DomTree, PostOrder}, context::Context, error::IrError, function::Function, @@ -20,6 +19,8 @@ use crate::{ LogEventData, Module, Pass, PassMutability, ScopedPass, StorageKey, TypeContent, TypeOption, UnaryOpKind, }; +use itertools::Itertools; +use rustc_hash::FxHashMap; pub struct ModuleVerifierResult; impl AnalysisResultT for ModuleVerifierResult {} @@ -45,6 +46,24 @@ pub fn create_module_verifier_pass() -> Pass { } } +struct SSADominanceScopeChecker { + post_order: PostOrder, + dom_tree: DomTree, + /// Position of each instruction within its block: `(block, index)`. + inst_index: FxHashMap, +} + +/// Maps each instruction to `(its block, its position within that block)`. +fn build_inst_index(context: &Context, function: Function) -> FxHashMap { + let mut idx = FxHashMap::default(); + for block in function.block_iter(context) { + for (pos, ins) in block.instruction_iter(context).enumerate() { + idx.insert(ins, (block, pos)); + } + } + idx +} + impl Context<'_> { /// Verify the contents of this [`Context`] is valid. pub fn verify(&self) -> Result<(), IrError> { @@ -256,9 +275,23 @@ impl Context<'_> { // } // } + let scope_checker = if self.verify_ssa_dominance { + let post_order = compute_post_order(self, &function); + let dom_tree = compute_dom_tree_from_po(self, function, &post_order); + let inst_index = build_inst_index(self, function); + Some(SSADominanceScopeChecker { + post_order, + dom_tree, + inst_index, + }) + } else { + None + }; + for block in function.block_iter(self) { - self.verify_block(cur_module, function, block)?; + self.verify_block(cur_module, function, block, scope_checker.as_ref())?; } + self.verify_metadata(function.get_metadata(self))?; Ok(()) } @@ -268,6 +301,7 @@ impl Context<'_> { cur_module: Module, cur_function: Function, cur_block: Block, + scope_checker: Option<&SSADominanceScopeChecker>, ) -> Result<(), IrError> { if cur_block.get_function(self) != cur_function { return Err(IrError::InconsistentParent( @@ -298,6 +332,7 @@ impl Context<'_> { cur_module, cur_function, cur_block, + scope_checker, } .verify_instructions(); @@ -390,10 +425,11 @@ struct InstructionVerifier<'a, 'eng> { cur_module: Module, cur_function: Function, cur_block: Block, + scope_checker: Option<&'a SSADominanceScopeChecker>, } impl InstructionVerifier<'_, '_> { - fn verify_instructions(&self) -> Result<(), IrError> { + fn verify_instructions(&mut self) -> Result<(), IrError> { for ins in self.cur_block.instruction_iter(self.context) { let value_content = &self.context.values[ins.0]; let ValueDatum::Instruction(instruction) = &value_content.value else { @@ -408,6 +444,18 @@ impl InstructionVerifier<'_, '_> { )); } + // SSA dominance check: operand must dominate ins + if let Some(scope) = self.scope_checker { + for operand in instruction.op.get_operands() { + if !self.check_def_dominates_use(scope, &operand, ins) { + return Err(IrError::VerifyInvalidScope { + value: self.value_to_string(&operand), + val: operand, + }); + } + } + } + match &instruction.op { InstOp::AsmBlock(..) => (), InstOp::BitCast(value, ty) => self.verify_bitcast(value, ty)?, @@ -1098,12 +1146,67 @@ impl InstructionVerifier<'_, '_> { Ok(()) } + fn value_to_string(&self, v: &Value) -> String { + if v.get_argument(self.context).is_some() { + if let Some(name) = self.cur_function.lookup_arg_name(self.context, v) { + return name.to_string(); + } + } + + format!("{:?}", v) + } + fn verify_load(&self, src_val: &Value) -> Result<(), IrError> { - // Just confirm `src_val` is a pointer. self.get_ptr_type(src_val, IrError::VerifyLoadFromNonPointer) .map(|_| ()) } + /// `def_value` block/argument/instruction must dominate `use_instruction_as_value`. + fn check_def_dominates_use( + &self, + scope: &SSADominanceScopeChecker, + def_value: &Value, + use_instruction_as_value: Value, + ) -> bool { + let Some(use_instruction) = use_instruction_as_value.get_instruction(self.context) else { + return true; + }; + let use_block = use_instruction.parent; + + // ignore checks inside unreachable blocks + if !scope.post_order.block_to_po.contains_key(&use_block) { + return true; + } + + // Constants are module-global; always in scope. + if def_value.is_constant(self.context) { + return true; + } + + // Block or function args dominates its own block and what it + // dominates. + if let Some(arg) = def_value.get_argument(self.context) { + return scope.dom_tree.dominates(arg.block, use_block); + } + + let Some(&(def_block, def_pos)) = scope.inst_index.get(def_value) else { + return true; + }; + + // def_block is unreachable, use_block is reachable, so we have + // a problem + if !scope.post_order.block_to_po.contains_key(&def_block) { + return false; + } + + if def_block == use_block { + let use_pos = scope.inst_index[&use_instruction_as_value].1; + def_pos <= use_pos + } else { + scope.dom_tree.dominates(def_block, use_block) + } + } + fn verify_alloc(&self, _ty: &Type, count: &Value) -> Result<(), IrError> { if matches!(count.get_type(self.context), Some(ty) if ty.is_uint64(self.context)) { Ok(()) diff --git a/sway-ir/tests/sroa/cross_block_gep_reuse.ir b/sway-ir/tests/sroa/cross_block_gep_reuse.ir new file mode 100644 index 00000000000..966cd93b088 --- /dev/null +++ b/sway-ir/tests/sroa/cross_block_gep_reuse.ir @@ -0,0 +1,23 @@ +// Regression test for an bug that was incorrectly scalarising the local `s` below. +// This test checks if SROA introduces a separate `get_local` for the scalarised field +// in each block that uses `e`. + +script { + entry fn main() -> u64 { + local { u64, u64 } s + + entry(): + vs = get_local __ptr { u64, u64 }, s + i0 = const u64 0 + e = get_elem_ptr vs, __ptr u64, i0 + c = const u64 42 + store c to e + x = load e + br next() + + next(): + y = load e + z = add x, y + ret u64 z + } +} diff --git a/sway-ir/tests/sroa/cross_block_gep_reuse.ir.snap b/sway-ir/tests/sroa/cross_block_gep_reuse.ir.snap new file mode 100644 index 00000000000..521be9823a7 --- /dev/null +++ b/sway-ir/tests/sroa/cross_block_gep_reuse.ir.snap @@ -0,0 +1,31 @@ +--- +source: sway-ir/tests/tests.rs +--- +Modified: true + +script { + entry fn main() -> u64 { + local { u64, u64 } s ++ local u64 s0 ++ local u64 s8 + + entry(): + v1v1 = get_local __ptr { u64, u64 }, s + v2v1 = const u64 0 + v3v1 = get_elem_ptr v1v1, __ptr u64, v2v1 ++ v11v1 = get_local __ptr u64, s0 + v4v1 = const u64 42 +- store v4v1 to v3v1 +- v6v1 = load v3v1 ++ store v4v1 to v11v1 ++ v6v1 = load v11v1 + br next() + + next(): +- v8v1 = load v3v1 ++ v12v1 = get_local __ptr u64, s0 ++ v8v1 = load v12v1 + v9v1 = add v6v1, v8v1 + ret u64 v9v1 + } +} diff --git a/sway-ir/tests/sroa/nested_struct_array.ir.snap b/sway-ir/tests/sroa/nested_struct_array.ir.snap index 39abaf3e6b8..a6559e2b86f 100644 --- a/sway-ir/tests/sroa/nested_struct_array.ir.snap +++ b/sway-ir/tests/sroa/nested_struct_array.ir.snap @@ -6,7 +6,9 @@ Modified: true script { entry fn main() -> u64, !1 { local [u64; 4] __anon_0 -- local { u64, u64 } __anon_1 + local { u64, u64 } __anon_1 ++ local u64 __anon_10 ++ local u64 __anon_18 local { u64, u64, [u64; 4], { u64, u64 } } __anon_2 entry(): @@ -27,83 +29,69 @@ script { v15v1 = get_elem_ptr v1v1, __ptr u64, v14v1 v16v1 = const u64 3, !6 store v16v1 to v15v1, !2 -- v18v1 = get_local __ptr { u64, u64 }, __anon_1, !7 -- v19v1 = const u64 0 -- v20v1 = get_elem_ptr v18v1, __ptr u64, v19v1 -- v21v1 = const u64 199, !8 + v18v1 = get_local __ptr { u64, u64 }, __anon_1, !7 + v19v1 = const u64 0 + v20v1 = get_elem_ptr v18v1, __ptr u64, v19v1 ++ v68v1 = get_local __ptr u64, __anon_10 + v21v1 = const u64 199, !8 - store v21v1 to v20v1, !7 -- v23v1 = const u64 1 -- v24v1 = get_elem_ptr v18v1, __ptr u64, v23v1 -- v25v1 = const u64 200, !9 ++ store v21v1 to v68v1, !7 + v23v1 = const u64 1 + v24v1 = get_elem_ptr v18v1, __ptr u64, v23v1 ++ v69v1 = get_local __ptr u64, __anon_18 + v25v1 = const u64 200, !9 - store v25v1 to v24v1, !7 -- v27v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !10 -+ v27v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !7 ++ store v25v1 to v69v1, !7 + v27v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !10 v28v1 = const u64 0 v29v1 = get_elem_ptr v27v1, __ptr u64, v28v1 -- v30v1 = const u64 2, !11 -- store v30v1 to v29v1, !10 -+ v30v1 = const u64 2, !8 -+ store v30v1 to v29v1, !7 + v30v1 = const u64 2, !11 + store v30v1 to v29v1, !10 v32v1 = const u64 1 v33v1 = get_elem_ptr v27v1, __ptr u64, v32v1 -- v34v1 = const u64 3, !12 -- store v34v1 to v33v1, !10 -+ v34v1 = const u64 3, !9 -+ store v34v1 to v33v1, !7 + v34v1 = const u64 3, !12 + store v34v1 to v33v1, !10 v36v1 = const u64 2 v37v1 = get_elem_ptr v27v1, __ptr [u64; 4], v36v1 mem_copy_val v37v1, v1v1 v39v1 = const u64 3 v40v1 = get_elem_ptr v27v1, __ptr { u64, u64 }, v39v1 - mem_copy_val v40v1, v18v1 -- v42v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !13 ++ v70v1 = get_local __ptr u64, __anon_10 ++ v71v1 = load v70v1 ++ v72v1 = get_local __ptr u64, __anon_18 ++ v73v1 = load v72v1 + v74v1 = const u64 0 + v75v1 = get_elem_ptr v40v1, __ptr u64, v74v1 -+ v21v1 = const u64 199, !10 -+ store v21v1 to v75v1 ++ store v71v1 to v75v1 + v77v1 = const u64 1 + v78v1 = get_elem_ptr v40v1, __ptr u64, v77v1 -+ v25v1 = const u64 200, !11 -+ store v25v1 to v78v1 -+ v42v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !12 ++ store v73v1 to v78v1 + v42v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !13 v43v1 = const u64 0 -- v44v1 = get_elem_ptr v42v1, __ptr u64, v43v1, !14 -+ v44v1 = get_elem_ptr v42v1, __ptr u64, v43v1, !13 + v44v1 = get_elem_ptr v42v1, __ptr u64, v43v1, !14 v45v1 = load v44v1 -- v46v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !15 -+ v46v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !14 + v46v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !15 v47v1 = const u64 3 -- v48v1 = get_elem_ptr v46v1, __ptr { u64, u64 }, v47v1, !16 -+ v48v1 = get_elem_ptr v46v1, __ptr { u64, u64 }, v47v1, !15 + v48v1 = get_elem_ptr v46v1, __ptr { u64, u64 }, v47v1, !16 v49v1 = const u64 0 -- v50v1 = get_elem_ptr v48v1, __ptr u64, v49v1, !17 -+ v50v1 = get_elem_ptr v48v1, __ptr u64, v49v1, !16 + v50v1 = get_elem_ptr v48v1, __ptr u64, v49v1, !17 v51v1 = load v50v1 -- v52v1 = add v45v1, v51v1, !18 -- v53v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !19 -+ v52v1 = add v45v1, v51v1, !17 -+ v53v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !18 + v52v1 = add v45v1, v51v1, !18 + v53v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !19 v54v1 = const u64 3 -- v55v1 = get_elem_ptr v53v1, __ptr { u64, u64 }, v54v1, !16 -+ v55v1 = get_elem_ptr v53v1, __ptr { u64, u64 }, v54v1, !15 + v55v1 = get_elem_ptr v53v1, __ptr { u64, u64 }, v54v1, !16 v56v1 = const u64 1 -- v57v1 = get_elem_ptr v55v1, __ptr u64, v56v1, !20 -+ v57v1 = get_elem_ptr v55v1, __ptr u64, v56v1, !19 + v57v1 = get_elem_ptr v55v1, __ptr u64, v56v1, !20 v58v1 = load v57v1 -- v59v1 = add v52v1, v58v1, !21 -- v60v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !22 -+ v59v1 = add v52v1, v58v1, !20 -+ v60v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !21 + v59v1 = add v52v1, v58v1, !21 + v60v1 = get_local __ptr { u64, u64, [u64; 4], { u64, u64 } }, __anon_2, !22 v61v1 = const u64 2 -- v62v1 = get_elem_ptr v60v1, __ptr [u64; 4], v61v1, !23 -- v63v1 = const u64 1, !24 -- v64v1 = get_elem_ptr v62v1, __ptr u64, v63v1, !25 -+ v62v1 = get_elem_ptr v60v1, __ptr [u64; 4], v61v1, !22 -+ v63v1 = const u64 1, !23 -+ v64v1 = get_elem_ptr v62v1, __ptr u64, v63v1, !24 + v62v1 = get_elem_ptr v60v1, __ptr [u64; 4], v61v1, !23 + v63v1 = const u64 1, !24 + v64v1 = get_elem_ptr v62v1, __ptr u64, v63v1, !25 v65v1 = load v64v1 -- v66v1 = add v59v1, v65v1, !26 -+ v66v1 = add v59v1, v65v1, !25 + v66v1 = add v59v1, v65v1, !26 ret u64 v66v1 } } @@ -115,42 +103,23 @@ script { !4 = span !0 178 179 !5 = span !0 181 182 !6 = span !0 184 185 -- !7 = span !0 199 220 -- !8 = span !0 206 209 -- !9 = span !0 215 218 -- !10 = span !0 130 226 -- !11 = span !0 146 147 -- !12 = span !0 160 161 -- !13 = span !0 232 233 -- !14 = span !0 56 62 -- !15 = span !0 238 239 -- !16 = span !0 91 95 -- !17 = span !0 22 28 -- !18 = span !0 232 243 -- !19 = span !0 246 247 -- !20 = span !0 32 38 -- !21 = span !0 232 251 -- !22 = span !0 254 255 -- !23 = span !0 76 87 -- !24 = span !0 258 259 -- !25 = span !0 254 260 -- !26 = span !0 232 260 -+ !7 = span !0 130 226 -+ !8 = span !0 146 147 -+ !9 = span !0 160 161 -+ !10 = span !0 206 209 -+ !11 = span !0 215 218 -+ !12 = span !0 232 233 -+ !13 = span !0 56 62 -+ !14 = span !0 238 239 -+ !15 = span !0 91 95 -+ !16 = span !0 22 28 -+ !17 = span !0 232 243 -+ !18 = span !0 246 247 -+ !19 = span !0 32 38 -+ !20 = span !0 232 251 -+ !21 = span !0 254 255 -+ !22 = span !0 76 87 -+ !23 = span !0 258 259 -+ !24 = span !0 254 260 -+ !25 = span !0 232 260 +!7 = span !0 199 220 +!8 = span !0 206 209 +!9 = span !0 215 218 +!10 = span !0 130 226 +!11 = span !0 146 147 +!12 = span !0 160 161 +!13 = span !0 232 233 +!14 = span !0 56 62 +!15 = span !0 238 239 +!16 = span !0 91 95 +!17 = span !0 22 28 +!18 = span !0 232 243 +!19 = span !0 246 247 +!20 = span !0 32 38 +!21 = span !0 232 251 +!22 = span !0 254 255 +!23 = span !0 76 87 +!24 = span !0 258 259 +!25 = span !0 254 260 +!26 = span !0 232 260 diff --git a/sway-ir/tests/sroa/struct_unused_field.ir.snap b/sway-ir/tests/sroa/struct_unused_field.ir.snap index 337a204533d..100ea8e26f2 100644 --- a/sway-ir/tests/sroa/struct_unused_field.ir.snap +++ b/sway-ir/tests/sroa/struct_unused_field.ir.snap @@ -5,33 +5,38 @@ Modified: true script { entry fn main() -> u64, !1 { -- local { u64, u64 } __anon_0 -- + local { u64, u64 } __anon_0 ++ local u64 __anon_00 ++ local u64 __anon_08 + entry(): -- v1v1 = get_local __ptr { u64, u64 }, __anon_0, !2 -- v2v1 = const u64 0 -- v3v1 = get_elem_ptr v1v1, __ptr u64, v2v1 -- v4v1 = const u64 2, !3 + v1v1 = get_local __ptr { u64, u64 }, __anon_0, !2 + v2v1 = const u64 0 + v3v1 = get_elem_ptr v1v1, __ptr u64, v2v1 ++ v15v1 = get_local __ptr u64, __anon_00 + v4v1 = const u64 2, !3 - store v4v1 to v3v1, !2 -- v6v1 = const u64 1 -- v7v1 = get_elem_ptr v1v1, __ptr u64, v6v1 -- v8v1 = const u64 3, !4 ++ store v4v1 to v15v1, !2 + v6v1 = const u64 1 + v7v1 = get_elem_ptr v1v1, __ptr u64, v6v1 ++ v16v1 = get_local __ptr u64, __anon_08 + v8v1 = const u64 3, !4 - store v8v1 to v7v1, !2 -- v10v1 = get_local __ptr { u64, u64 }, __anon_0, !5 -- v11v1 = const u64 0 -- v12v1 = get_elem_ptr v10v1, __ptr u64, v11v1, !6 ++ store v8v1 to v16v1, !2 + v10v1 = get_local __ptr { u64, u64 }, __anon_0, !5 + v11v1 = const u64 0 + v12v1 = get_elem_ptr v10v1, __ptr u64, v11v1, !6 - v13v1 = load v12v1 -- ret u64 v13v1 -+ v4v1 = const u64 2, !2 -+ ret u64 v4v1 ++ v17v1 = get_local __ptr u64, __anon_00 ++ v13v1 = load v17v1 + ret u64 v13v1 } } !0 = "test/src/main.sw" !1 = span !0 43 122 -- !2 = span !0 73 111 -- !3 = span !0 89 90 -- !4 = span !0 103 104 -- !5 = span !0 117 118 -- !6 = span !0 22 28 -+ !2 = span !0 89 90 +!2 = span !0 73 111 +!3 = span !0 89 90 +!4 = span !0 103 104 +!5 = span !0 117 118 +!6 = span !0 22 28 diff --git a/sway-ir/tests/tests.rs b/sway-ir/tests/tests.rs index 0d0f436e996..51122a94136 100644 --- a/sway-ir/tests/tests.rs +++ b/sway-ir/tests/tests.rs @@ -1,5 +1,6 @@ use std::{ any::Any, + collections::HashSet, panic::catch_unwind, path::{Path, PathBuf}, }; @@ -13,9 +14,8 @@ use sway_ir::{ create_mem2reg_pass, create_memcpyopt_pass, create_memcpyprop_reverse_pass, create_misc_demotion_pass, create_postorder_pass, create_ret_demotion_pass, create_simplify_cfg_pass, metadata_to_inline, optimize as opt, register_known_passes, - Backtrace, Context, Function, IrError, PassGroup, PassManager, Value, DCE_NAME, - FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, GLOBALS_DCE_NAME, MEM2REG_NAME, - SROA_NAME, + Backtrace, Context, Function, IrError, Options, PassGroup, PassManager, Value, + FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, GLOBALS_DCE_NAME, SROA_NAME, }; use sway_types::SourceEngine; @@ -191,6 +191,29 @@ fn run_tests bool>(sub_dir: &str, opt_fn: F) { } } +fn run_passes_with_verify( + pass_mgr: &mut PassManager, + ir: &mut Context, + passes: &PassGroup, +) -> bool { + ir.verify_ssa_dominance = true; + pass_mgr + .run( + ir, + passes, + &Options { + print_initial: false, + print_final: false, + print_modified_only: false, + print_metadata: false, + print_passes: HashSet::default(), + force_verify_ir: true, + rounds: 1, // we want to check the effect of the optimization only once + }, + ) + .unwrap() +} + // Utility for finding test files and running IR verifier tests. // Each test file must contain an IR code that is parsable, // but does not pass IR verification. @@ -348,7 +371,7 @@ fn constants() { let mut pass_group = PassGroup::default(); let pass = pass_mgr.register(create_const_folding_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -364,7 +387,7 @@ fn ccp() { pass_mgr.register(create_dominators_pass()); let pass = pass_mgr.register(create_ccp_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -378,7 +401,7 @@ fn simplify_cfg() { let mut pass_group = PassGroup::default(); let pass = pass_mgr.register(create_simplify_cfg_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -398,7 +421,7 @@ fn dce() { // Some tests require multiple passes of DCE to be run, // this also reflects our actual compiler pipeline where DCE runs multiple times. pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -414,7 +437,7 @@ fn cse() { pass_mgr.register(create_dominators_pass()); let pass = pass_mgr.register(create_cse_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -431,7 +454,7 @@ fn mem2reg() { pass_mgr.register(create_dom_fronts_pass()); let pass = pass_mgr.register(create_mem2reg_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -445,7 +468,7 @@ fn demote_arg() { let mut pass_group = PassGroup::default(); let pass = pass_mgr.register(create_arg_demotion_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -459,7 +482,7 @@ fn demote_const() { let mut pass_group = PassGroup::default(); let pass = pass_mgr.register(create_const_demotion_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -473,7 +496,7 @@ fn demote_ret() { let mut pass_group = PassGroup::default(); let pass = pass_mgr.register(create_ret_demotion_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -487,7 +510,7 @@ fn demote_misc() { let mut pass_group = PassGroup::default(); let pass = pass_mgr.register(create_misc_demotion_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -504,7 +527,7 @@ fn memcpyopt() { pass_mgr.register(create_escaped_symbols_pass()); let pass = pass_mgr.register(create_memcpyopt_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -518,7 +541,7 @@ fn memcpy_prop() { let mut pass_group = PassGroup::default(); let pass = pass_mgr.register(create_memcpyprop_reverse_pass()); pass_group.append_pass(pass); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -532,9 +555,7 @@ fn sroa() { let mut pass_group = PassGroup::default(); register_known_passes(&mut pass_mgr); pass_group.append_pass(SROA_NAME); - pass_group.append_pass(MEM2REG_NAME); - pass_group.append_pass(DCE_NAME); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -548,7 +569,7 @@ fn globals_dce() { let mut pass_group = PassGroup::default(); register_known_passes(&mut pass_mgr); pass_group.append_pass(GLOBALS_DCE_NAME); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -563,7 +584,7 @@ fn fndedup_debug() { register_known_passes(&mut pass_mgr); pass_group.append_pass(FN_DEDUP_DEBUG_PROFILE_NAME); pass_group.append_pass(GLOBALS_DCE_NAME); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } @@ -576,7 +597,7 @@ fn fndedup_release() { register_known_passes(&mut pass_mgr); pass_group.append_pass(FN_DEDUP_RELEASE_PROFILE_NAME); pass_group.append_pass(GLOBALS_DCE_NAME); - pass_mgr.run(ir, &pass_group).unwrap() + run_passes_with_verify(&mut pass_mgr, ir, &pass_group) }) } diff --git a/test/src/ir_generation/mod.rs b/test/src/ir_generation/mod.rs index 732dc884143..ac43e4271f0 100644 --- a/test/src/ir_generation/mod.rs +++ b/test/src/ir_generation/mod.rs @@ -16,7 +16,7 @@ use sway_error::handler::Handler; use sway_features::ExperimentalFeatures; use sway_ir::{ - create_fn_inline_pass, register_known_passes, Backtrace, PassGroup, PassManager, + create_fn_inline_pass, register_known_passes, Backtrace, Options, PassGroup, PassManager, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, DCE_NAME, INIT_AGGR_LOWERING_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, }; @@ -320,7 +320,10 @@ pub(super) async fn run( pass_group.append_pass(MISC_DEMOTION_NAME); pass_group.append_pass(MEMCPYOPT_NAME); pass_group.append_pass(DCE_NAME); - if pass_mgr.run(&mut ir, &pass_group).is_err() { + if pass_mgr + .run(&mut ir, &pass_group, &Options::default()) + .is_err() + { panic!( "Failed to compile test {}:\n{}", path.display(), @@ -393,7 +396,8 @@ pub(super) async fn run( ) .unwrap_or_else(|e| panic!("{}: {e}\n{ir_output}", path.display())); - let _ = pass_mgr.run(&mut ir, &group); + let _ = pass_mgr + .run(&mut ir, &group, &Options::default()); let ir_output = sway_ir::printer::to_string(&ir); match checker.explain(&ir_output, filecheck::NO_VARIABLES) @@ -419,7 +423,11 @@ pub(super) async fn run( let mut pmgr_config = PassGroup::default(); let inline = pass_mgr.register(create_fn_inline_pass()); pmgr_config.append_pass(inline); - let inline_res = pass_mgr.run(&mut ir, &pmgr_config); + let inline_res = pass_mgr.run( + &mut ir, + &pmgr_config, + &Options::default(), + ); if inline_res.is_err() { panic!( "Failed to compile test {}:\n{}",