From 2856781f68da028925fa83772693ed088bb090aa Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Tue, 21 Jul 2026 15:47:25 -0300 Subject: [PATCH 1/6] fix sroa bug causing use-before-def of some values --- sway-ir/src/error.rs | 9 ++ sway-ir/src/optimize/sroa.rs | 16 +- sway-ir/src/verify.rs | 48 +++++- sway-ir/tests/sroa/cross_block_gep_reuse.ir | 23 +++ .../tests/sroa/cross_block_gep_reuse.ir.snap | 33 ++++ .../tests/sroa/nested_struct_array.ir.snap | 153 +++++++----------- .../tests/sroa/struct_unused_field.ir.snap | 49 +++--- sway-ir/tests/tests.rs | 60 ++++--- 8 files changed, 249 insertions(+), 142 deletions(-) create mode 100644 sway-ir/tests/sroa/cross_block_gep_reuse.ir create mode 100644 sway-ir/tests/sroa/cross_block_gep_reuse.ir.snap diff --git a/sway-ir/src/error.rs b/sway-ir/src/error.rs index ef77d3e9585..ef9b546a099 100644 --- a/sway-ir/src/error.rs +++ b/sway-ir/src/error.rs @@ -98,6 +98,9 @@ pub enum IrError { VerifyInitAggrUnknownInitializerType(usize), VerifyInitAggrMismatchedStructFieldType(usize, String, String), VerifyInitAggrMismatchedArrayElementType(usize, String, String), + VerifyUnknownValue { + value: String, + }, } impl IrError { @@ -603,6 +606,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::VerifyUnknownValue { value } => { + write!( + f, + "Verification failed: unknown value: {value}", + ) + }, } } } 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/verify.rs b/sway-ir/src/verify.rs index 926d2923737..42af9a3a563 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -3,8 +3,6 @@ //! 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::{ context::Context, error::IrError, @@ -19,6 +17,8 @@ use crate::{ BranchToWithArgs, Config, Doc, GlobalVar, InitAggr, LogEventData, Module, Pass, PassMutability, ScopedPass, StorageKey, TypeContent, TypeOption, UnaryOpKind, }; +use itertools::Itertools; +use std::collections::HashSet; pub struct ModuleVerifierResult; impl AnalysisResultT for ModuleVerifierResult {} @@ -121,9 +121,28 @@ impl Context<'_> { // } // } + let mut declared = HashSet::default(); + for arg in function.args_iter(self) { + declared.insert(arg.value); + } + for block in function.block_iter(self) { - self.verify_block(cur_module, function, block)?; + let is_entry = block.is_entry(self); + + for arg in block.arg_iter(self) { + declared.insert(*arg); + } + + self.verify_block(cur_module, function, block, &mut declared)?; + + // Otherwise we would remove function args + if !is_entry { + for arg in block.arg_iter(self) { + declared.remove(arg); + } + } } + self.verify_metadata(function.get_metadata(self))?; Ok(()) } @@ -133,6 +152,7 @@ impl Context<'_> { cur_module: Module, cur_function: Function, cur_block: Block, + declared: &mut HashSet, ) -> Result<(), IrError> { if cur_block.get_function(self) != cur_function { return Err(IrError::InconsistentParent( @@ -163,6 +183,7 @@ impl Context<'_> { cur_module, cur_function, cur_block, + declared, } .verify_instructions(); @@ -255,10 +276,11 @@ struct InstructionVerifier<'a, 'eng> { cur_module: Module, cur_function: Function, cur_block: Block, + declared: &'a mut HashSet, } 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 { @@ -273,6 +295,8 @@ impl InstructionVerifier<'_, '_> { )); } + self.declared.insert(ins); + match &instruction.op { InstOp::AsmBlock(..) => (), InstOp::BitCast(value, ty) => self.verify_bitcast(value, ty)?, @@ -963,7 +987,23 @@ 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> { + if !self.declared.contains(src_val) { + return Err(IrError::VerifyUnknownValue { + value: self.value_to_string(src_val), + }); + } + // Just confirm `src_val` is a pointer. self.get_ptr_type(src_val, IrError::VerifyLoadFromNonPointer) .map(|_| ()) 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..2249048e5b7 --- /dev/null +++ b/sway-ir/tests/sroa/cross_block_gep_reuse.ir.snap @@ -0,0 +1,33 @@ +--- +source: sway-ir/tests/tests.rs +--- +Modified: true + +script { + entry fn main() -> u64 { + local { u64, u64 } s ++ local u64 s0 ++ local u64 s00 ++ local u64 s8 ++ local u64 s80 + + 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..27cd4100805 100644 --- a/sway-ir/tests/sroa/nested_struct_array.ir.snap +++ b/sway-ir/tests/sroa/nested_struct_array.ir.snap @@ -6,7 +6,11 @@ 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_100 ++ local u64 __anon_18 ++ local u64 __anon_180 local { u64, u64, [u64; 4], { u64, u64 } } __anon_2 entry(): @@ -27,83 +31,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 +105,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..d5e894f8688 100644 --- a/sway-ir/tests/sroa/struct_unused_field.ir.snap +++ b/sway-ir/tests/sroa/struct_unused_field.ir.snap @@ -5,33 +5,40 @@ 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_000 ++ local u64 __anon_08 ++ local u64 __anon_080 + 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..9e11bd9d741 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, PassGroup, PassManager, PrintPassesOpts, Value, + FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, GLOBALS_DCE_NAME, SROA_NAME, }; use sway_types::SourceEngine; @@ -191,6 +191,26 @@ 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 { + pass_mgr + .run_with_print_verify( + ir, + passes, + &PrintPassesOpts { + initial: false, + r#final: false, + modified_only: false, + metadata: false, + passes: HashSet::default(), + }, + ) + .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 +368,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 +384,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 +398,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 +418,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 +434,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 +451,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 +465,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 +479,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 +493,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 +507,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 +524,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 +538,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 +552,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 +566,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 +581,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 +594,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) }) } From 4d77edc8ecb84c055add7030d7a2207c896706d0 Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Tue, 21 Jul 2026 15:50:10 -0300 Subject: [PATCH 2/6] fix SROA bug when there are loads from multiple blocks --- sway-core/src/lib.rs | 3 + sway-ir/src/analysis/dominator.rs | 21 ++++-- sway-ir/src/context.rs | 6 ++ sway-ir/src/error.rs | 6 +- sway-ir/src/verify.rs | 120 ++++++++++++++++++++++-------- sway-ir/tests/tests.rs | 1 + 6 files changed, 117 insertions(+), 40 deletions(-) diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs index fec21d2ca0a..31b6dc6c939 100644 --- a/sway-core/src/lib.rs +++ b/sway-core/src/lib.rs @@ -1615,6 +1615,9 @@ pub(crate) fn compile_ast_to_ir_to_asm( // Run the passes. let print_passes_opts: PrintPassesOpts = (&build_config.print_ir).into(); + ir.verify_ssa_dominance = std::env::var("SWAY_FORCE_VERIFY_IR") + .map(|v| v.parse::().unwrap_or(false)) + .unwrap_or(false); let res = if let Err(ir_error) = pass_mgr.run_with_print_verify(&mut ir, &pass_group, &print_passes_opts) { diff --git a/sway-ir/src/analysis/dominator.rs b/sway-ir/src/analysis/dominator.rs index 4d65b53f4d3..ccd91781844 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 funtion, 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/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 ef9b546a099..95c4684603c 100644 --- a/sway-ir/src/error.rs +++ b/sway-ir/src/error.rs @@ -98,8 +98,9 @@ pub enum IrError { VerifyInitAggrUnknownInitializerType(usize), VerifyInitAggrMismatchedStructFieldType(usize, String, String), VerifyInitAggrMismatchedArrayElementType(usize, String, String), - VerifyUnknownValue { + VerifyInvalidScope { value: String, + val: Value, }, } @@ -109,6 +110,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, } } @@ -606,7 +608,7 @@ 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::VerifyUnknownValue { value } => { + IrError::VerifyInvalidScope { value, .. } => { write!( f, "Verification failed: unknown value: {value}", diff --git a/sway-ir/src/verify.rs b/sway-ir/src/verify.rs index 42af9a3a563..fb550e15c66 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -4,6 +4,7 @@ //! consistent valid state, using the functions in this module. use crate::{ + analysis::dominator::{compute_dom_tree_from_po, compute_post_order, DomTree, PostOrder}, context::Context, error::IrError, function::Function, @@ -18,7 +19,7 @@ use crate::{ ScopedPass, StorageKey, TypeContent, TypeOption, UnaryOpKind, }; use itertools::Itertools; -use std::collections::HashSet; +use rustc_hash::FxHashMap; pub struct ModuleVerifierResult; impl AnalysisResultT for ModuleVerifierResult {} @@ -44,6 +45,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> { @@ -121,26 +140,21 @@ impl Context<'_> { // } // } - let mut declared = HashSet::default(); - for arg in function.args_iter(self) { - declared.insert(arg.value); - } + 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) { - let is_entry = block.is_entry(self); - - for arg in block.arg_iter(self) { - declared.insert(*arg); - } - - self.verify_block(cur_module, function, block, &mut declared)?; - - // Otherwise we would remove function args - if !is_entry { - for arg in block.arg_iter(self) { - declared.remove(arg); - } - } + self.verify_block(cur_module, function, block, scope_checker.as_ref())?; } self.verify_metadata(function.get_metadata(self))?; @@ -152,7 +166,7 @@ impl Context<'_> { cur_module: Module, cur_function: Function, cur_block: Block, - declared: &mut HashSet, + scope_checker: Option<&SSADominanceScopeChecker>, ) -> Result<(), IrError> { if cur_block.get_function(self) != cur_function { return Err(IrError::InconsistentParent( @@ -183,7 +197,7 @@ impl Context<'_> { cur_module, cur_function, cur_block, - declared, + scope_checker, } .verify_instructions(); @@ -276,7 +290,7 @@ struct InstructionVerifier<'a, 'eng> { cur_module: Module, cur_function: Function, cur_block: Block, - declared: &'a mut HashSet, + scope_checker: Option<&'a SSADominanceScopeChecker>, } impl InstructionVerifier<'_, '_> { @@ -295,8 +309,6 @@ impl InstructionVerifier<'_, '_> { )); } - self.declared.insert(ins); - match &instruction.op { InstOp::AsmBlock(..) => (), InstOp::BitCast(value, ty) => self.verify_bitcast(value, ty)?, @@ -414,7 +426,7 @@ impl InstructionVerifier<'_, '_> { InstOp::GetConfig(config) => self.verify_get_config(config)?, InstOp::GetStorageKey(storage_key) => self.verify_get_storage_key(storage_key)?, InstOp::IntToPtr(value, ty) => self.verify_int_to_ptr(value, ty)?, - InstOp::Load(ptr) => self.verify_load(ptr)?, + InstOp::Load(ptr) => self.verify_load(ptr, ins)?, InstOp::Alloc { ty, count } => self.verify_alloc(ty, count)?, InstOp::MemCopyBytes { dst_val_ptr, @@ -997,11 +1009,14 @@ impl InstructionVerifier<'_, '_> { format!("{:?}", v) } - fn verify_load(&self, src_val: &Value) -> Result<(), IrError> { - if !self.declared.contains(src_val) { - return Err(IrError::VerifyUnknownValue { - value: self.value_to_string(src_val), - }); + fn verify_load(&self, src_val: &Value, use_ins: Value) -> Result<(), IrError> { + if let Some(scope_checker) = self.scope_checker.as_ref() { + if !self.check_value_scope(scope_checker, src_val, use_ins) { + return Err(IrError::VerifyInvalidScope { + value: self.value_to_string(src_val), + val: *src_val, + }); + } } // Just confirm `src_val` is a pointer. @@ -1009,6 +1024,51 @@ impl InstructionVerifier<'_, '_> { .map(|_| ()) } + fn check_value_scope( + &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 unreacheable, use_block is reacheable, 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/tests.rs b/sway-ir/tests/tests.rs index 9e11bd9d741..58a06f3281e 100644 --- a/sway-ir/tests/tests.rs +++ b/sway-ir/tests/tests.rs @@ -196,6 +196,7 @@ fn run_passes_with_verify( ir: &mut Context, passes: &PassGroup, ) -> bool { + ir.verify_ssa_dominance = true; pass_mgr .run_with_print_verify( ir, From 495007f40c411c906fdfda474713c4492776a15b Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Tue, 21 Jul 2026 14:55:45 -0300 Subject: [PATCH 3/6] remove PassManager::run --- sway-ir/src/bin/opt.rs | 4 ++-- sway-ir/src/optimize.rs | 6 ++++-- sway-ir/src/pass_manager.rs | 11 +---------- test/src/ir_generation/mod.rs | 18 +++++++++++++----- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/sway-ir/src/bin/opt.rs b/sway-ir/src/bin/opt.rs index ef2b69b45c5..efef908f109 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, PassGroup, PassManager, PrintPassesOpts, 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_with_print_verify(&mut ir, &passes, &PrintPassesOpts::default())?; // Write the output file or standard out. write_to_output(ir, &config.output_path)?; diff --git a/sway-ir/src/optimize.rs b/sway-ir/src/optimize.rs index 72c2136a04d..fefe6c0d33d 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, PassGroup, PassManager, PrintPassesOpts}; use sway_features::ExperimentalFeatures; use sway_types::SourceEngine; @@ -104,7 +104,9 @@ pub mod tests { } let before = context.to_string(); - let modified = pass_manager.run(&mut context, &group).unwrap(); + let modified = pass_manager + .run_with_print_verify(&mut context, &group, &PrintPassesOpts::default()) + .unwrap(); let after = context.to_string(); // print diff to help debug diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index 2177c9ee542..afc058f6395 100644 --- a/sway-ir/src/pass_manager.rs +++ b/sway-ir/src/pass_manager.rs @@ -162,7 +162,7 @@ impl AnalysisResults { /// 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)] +#[derive(Debug, Default)] pub struct PrintPassesOpts { pub initial: bool, pub r#final: bool, @@ -363,15 +363,6 @@ 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( diff --git a/test/src/ir_generation/mod.rs b/test/src/ir_generation/mod.rs index 732dc884143..5e8042a3922 100644 --- a/test/src/ir_generation/mod.rs +++ b/test/src/ir_generation/mod.rs @@ -17,8 +17,8 @@ use sway_error::handler::Handler; use sway_features::ExperimentalFeatures; use sway_ir::{ create_fn_inline_pass, register_known_passes, Backtrace, PassGroup, PassManager, - ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, DCE_NAME, INIT_AGGR_LOWERING_NAME, MEMCPYOPT_NAME, - MISC_DEMOTION_NAME, RET_DEMOTION_NAME, + PrintPassesOpts, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, DCE_NAME, INIT_AGGR_LOWERING_NAME, + MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, }; use sway_types::ProgramId; @@ -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_with_print_verify(&mut ir, &pass_group, &PrintPassesOpts::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_with_print_verify(&mut ir, &group, &PrintPassesOpts::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_with_print_verify( + &mut ir, + &pmgr_config, + &PrintPassesOpts::default(), + ); if inline_res.is_err() { panic!( "Failed to compile test {}:\n{}", From b19ed368d9dc97bc1611873d95b64e27acee9f1c Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Tue, 21 Jul 2026 15:51:28 -0300 Subject: [PATCH 4/6] fix rebase issues --- sway-ir/src/verify.rs | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/sway-ir/src/verify.rs b/sway-ir/src/verify.rs index fb550e15c66..5e12ba3e2db 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -309,6 +309,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)?, @@ -426,7 +438,7 @@ impl InstructionVerifier<'_, '_> { InstOp::GetConfig(config) => self.verify_get_config(config)?, InstOp::GetStorageKey(storage_key) => self.verify_get_storage_key(storage_key)?, InstOp::IntToPtr(value, ty) => self.verify_int_to_ptr(value, ty)?, - InstOp::Load(ptr) => self.verify_load(ptr, ins)?, + InstOp::Load(ptr) => self.verify_load(ptr)?, InstOp::Alloc { ty, count } => self.verify_alloc(ty, count)?, InstOp::MemCopyBytes { dst_val_ptr, @@ -1009,22 +1021,13 @@ impl InstructionVerifier<'_, '_> { format!("{:?}", v) } - fn verify_load(&self, src_val: &Value, use_ins: Value) -> Result<(), IrError> { - if let Some(scope_checker) = self.scope_checker.as_ref() { - if !self.check_value_scope(scope_checker, src_val, use_ins) { - return Err(IrError::VerifyInvalidScope { - value: self.value_to_string(src_val), - val: *src_val, - }); - } - } - - // Just confirm `src_val` is a pointer. + fn verify_load(&self, src_val: &Value) -> Result<(), IrError> { self.get_ptr_type(src_val, IrError::VerifyLoadFromNonPointer) .map(|_| ()) } - fn check_value_scope( + /// `def_value` block/argument/instruction must dominate `use_instruction_as_value`. + fn check_def_dominates_use( &self, scope: &SSADominanceScopeChecker, def_value: &Value, From 2bc1bb3ec4f4ccd2d122dfafe59bd61ed88148f8 Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Tue, 21 Jul 2026 17:23:36 -0300 Subject: [PATCH 5/6] fix typos --- sway-ir/src/analysis/dominator.rs | 2 +- sway-ir/src/verify.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sway-ir/src/analysis/dominator.rs b/sway-ir/src/analysis/dominator.rs index ccd91781844..d7f416524b9 100644 --- a/sway-ir/src/analysis/dominator.rs +++ b/sway-ir/src/analysis/dominator.rs @@ -111,7 +111,7 @@ pub fn create_dominators_pass() -> Pass { } } -/// Compute the dominator tree of the funtion, given a post-order traversal. +/// 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); diff --git a/sway-ir/src/verify.rs b/sway-ir/src/verify.rs index 5e12ba3e2db..4f36e7ec37f 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -1058,7 +1058,7 @@ impl InstructionVerifier<'_, '_> { return true; }; - // def_block is unreacheable, use_block is reacheable, so we have + // 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; From 6c4bd1884fdc10911ae88945742fa67249a68d34 Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Wed, 22 Jul 2026 09:19:24 -0300 Subject: [PATCH 6/6] make optimizations verify and rounds part of its options --- sway-core/src/build_config.rs | 15 +++-- sway-core/src/lib.rs | 16 +++-- sway-ir/src/bin/opt.rs | 4 +- sway-ir/src/optimize.rs | 11 +++- sway-ir/src/pass_manager.rs | 64 +++++++++++-------- .../tests/sroa/cross_block_gep_reuse.ir.snap | 2 - .../tests/sroa/nested_struct_array.ir.snap | 2 - .../tests/sroa/struct_unused_field.ir.snap | 2 - sway-ir/tests/tests.rs | 18 +++--- test/src/ir_generation/mod.rs | 14 ++-- 10 files changed, 84 insertions(+), 64 deletions(-) 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 31b6dc6c939..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,13 +1614,15 @@ pub(crate) fn compile_ast_to_ir_to_asm( } // Run the passes. - let print_passes_opts: PrintPassesOpts = (&build_config.print_ir).into(); - ir.verify_ssa_dominance = std::env::var("SWAY_FORCE_VERIFY_IR") + 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); - let res = if let Err(ir_error) = - pass_mgr.run_with_print_verify(&mut ir, &pass_group, &print_passes_opts) - { + 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/bin/opt.rs b/sway-ir/src/bin/opt.rs index efef908f109..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, PrintPassesOpts, + 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_with_print_verify(&mut ir, &passes, &PrintPassesOpts::default())?; + 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/optimize.rs b/sway-ir/src/optimize.rs index fefe6c0d33d..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, PrintPassesOpts}; + use crate::{Backtrace, Options, PassGroup, PassManager}; use sway_features::ExperimentalFeatures; use sway_types::SourceEngine; @@ -105,7 +105,14 @@ pub mod tests { let before = context.to_string(); let modified = pass_manager - .run_with_print_verify(&mut context, &group, &PrintPassesOpts::default()) + .run( + &mut context, + &group, + &Options { + rounds: 1, + ..Default::default() + }, + ) .unwrap(); let after = context.to_string(); diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index afc058f6395..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, Default)] -pub struct PrintPassesOpts { - pub initial: bool, - pub r#final: bool, - pub modified_only: bool, - pub metadata: bool, - pub passes: HashSet, +#[derive(Debug)] +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. @@ -365,14 +383,14 @@ impl PassManager { /// 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 @@ -380,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() @@ -400,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() @@ -408,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 { @@ -437,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/tests/sroa/cross_block_gep_reuse.ir.snap b/sway-ir/tests/sroa/cross_block_gep_reuse.ir.snap index 2249048e5b7..521be9823a7 100644 --- a/sway-ir/tests/sroa/cross_block_gep_reuse.ir.snap +++ b/sway-ir/tests/sroa/cross_block_gep_reuse.ir.snap @@ -7,9 +7,7 @@ script { entry fn main() -> u64 { local { u64, u64 } s + local u64 s0 -+ local u64 s00 + local u64 s8 -+ local u64 s80 entry(): v1v1 = get_local __ptr { u64, u64 }, s diff --git a/sway-ir/tests/sroa/nested_struct_array.ir.snap b/sway-ir/tests/sroa/nested_struct_array.ir.snap index 27cd4100805..a6559e2b86f 100644 --- a/sway-ir/tests/sroa/nested_struct_array.ir.snap +++ b/sway-ir/tests/sroa/nested_struct_array.ir.snap @@ -8,9 +8,7 @@ script { local [u64; 4] __anon_0 local { u64, u64 } __anon_1 + local u64 __anon_10 -+ local u64 __anon_100 + local u64 __anon_18 -+ local u64 __anon_180 local { u64, u64, [u64; 4], { u64, u64 } } __anon_2 entry(): diff --git a/sway-ir/tests/sroa/struct_unused_field.ir.snap b/sway-ir/tests/sroa/struct_unused_field.ir.snap index d5e894f8688..100ea8e26f2 100644 --- a/sway-ir/tests/sroa/struct_unused_field.ir.snap +++ b/sway-ir/tests/sroa/struct_unused_field.ir.snap @@ -7,9 +7,7 @@ script { entry fn main() -> u64, !1 { local { u64, u64 } __anon_0 + local u64 __anon_00 -+ local u64 __anon_000 + local u64 __anon_08 -+ local u64 __anon_080 entry(): v1v1 = get_local __ptr { u64, u64 }, __anon_0, !2 diff --git a/sway-ir/tests/tests.rs b/sway-ir/tests/tests.rs index 58a06f3281e..51122a94136 100644 --- a/sway-ir/tests/tests.rs +++ b/sway-ir/tests/tests.rs @@ -14,7 +14,7 @@ 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, PrintPassesOpts, Value, + 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; @@ -198,15 +198,17 @@ fn run_passes_with_verify( ) -> bool { ir.verify_ssa_dominance = true; pass_mgr - .run_with_print_verify( + .run( ir, passes, - &PrintPassesOpts { - initial: false, - r#final: false, - modified_only: false, - metadata: false, - passes: HashSet::default(), + &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() diff --git a/test/src/ir_generation/mod.rs b/test/src/ir_generation/mod.rs index 5e8042a3922..ac43e4271f0 100644 --- a/test/src/ir_generation/mod.rs +++ b/test/src/ir_generation/mod.rs @@ -16,9 +16,9 @@ use sway_error::handler::Handler; use sway_features::ExperimentalFeatures; use sway_ir::{ - create_fn_inline_pass, register_known_passes, Backtrace, PassGroup, PassManager, - PrintPassesOpts, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, DCE_NAME, INIT_AGGR_LOWERING_NAME, - MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, + 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, }; use sway_types::ProgramId; @@ -321,7 +321,7 @@ pub(super) async fn run( pass_group.append_pass(MEMCPYOPT_NAME); pass_group.append_pass(DCE_NAME); if pass_mgr - .run_with_print_verify(&mut ir, &pass_group, &PrintPassesOpts::default()) + .run(&mut ir, &pass_group, &Options::default()) .is_err() { panic!( @@ -397,7 +397,7 @@ pub(super) async fn run( .unwrap_or_else(|e| panic!("{}: {e}\n{ir_output}", path.display())); let _ = pass_mgr - .run_with_print_verify(&mut ir, &group, &PrintPassesOpts::default()); + .run(&mut ir, &group, &Options::default()); let ir_output = sway_ir::printer::to_string(&ir); match checker.explain(&ir_output, filecheck::NO_VARIABLES) @@ -423,10 +423,10 @@ 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_with_print_verify( + let inline_res = pass_mgr.run( &mut ir, &pmgr_config, - &PrintPassesOpts::default(), + &Options::default(), ); if inline_res.is_err() { panic!(