Fix SROA when loading in multiple blocks

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()
         }
     }
 }

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::<bool>().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(),

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<AnalysisResult, IrError> {
-    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<AnalysisResult, IrError> {
+    let po: &PostOrder = analyses.get_analysis_result(function);
+    Ok(Box::new(compute_dom_tree_from_po(context, function, po)))
 }
 
 impl DomTree {

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)?;

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

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}",
+                )
+            },
         }
     }
 }

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

sway-ir/src/optimize/sroa.rs

@@ -128,10 +128,9 @@ pub fn sroa(
         })
         .collect();
 
-    let mut scalar_replacements = FxHashMap::<Value, Value>::default();
-
     for block in function.block_iter(context) {
         let mut new_insts = Vec::new();
+        let mut replacements_per_block = FxHashMap::<Value, Value>::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)
 }

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<String>,
+pub struct Options {
+    pub print_initial: bool,
+    pub print_final: bool,
+    pub print_modified_only: bool,
+    pub print_metadata: bool,
+    pub print_passes: HashSet<String>,
+    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,43 +381,29 @@ 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<bool, IrError> {
-        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<bool, IrError> {
-        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
         ir.verify()?;
 
         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,25 +413,26 @@ 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()
                 };
 
                 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)