fix modified in optimisation passes so they converge

forc-test/src/execute.rs

@@ -85,9 +85,8 @@ impl TestExecutor {
             .add_unsigned_coin_input(secret_key, utxo_id, amount, asset_id, tx_pointer)
             .maturity(maturity);
 
-        let mut output_index = 1;
         // Insert contract ids into tx input
-        for contract_id in test_setup.contract_ids() {
+        for (output_index, contract_id) in (1..).zip(test_setup.contract_ids()) {
             tx_builder
                 .add_input(tx::Input::contract(
                     tx::UtxoId::new(tx::Bytes32::zeroed(), 0),
@@ -101,7 +100,6 @@ impl TestExecutor {
                     balance_root: fuel_tx::Bytes32::zeroed(),
                     state_root: tx::Bytes32::zeroed(),
                 }));
-            output_index += 1;
         }
 
         let consensus_params = tx_builder.get_params().clone();

sway-core/src/asm_generation/fuel/fuel_asm_builder.rs

@@ -2335,7 +2335,7 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> {
         // XXX not required after we have FuelVM specific verifier.
         if !val
             .get_type(self.context)
-            .and_then(|val_ty| key.get_type(self.context).map(|key_ty| (val_ty, key_ty)))
+            .zip(key.get_type(self.context))
             .is_some_and(|(val_ty, key_ty)| {
                 val_ty.is_ptr(self.context) && key_ty.is_ptr(self.context)
             })

sway-core/src/asm_generation/fuel/functions.rs

@@ -418,7 +418,7 @@ impl FuelAsmBuilder<'_, '_> {
         let fn_name = function.get_name(self.context);
         let uses_stack =
             function.num_args(self.context) > compiler_constants::NUM_ARG_REGISTERS as usize;
-        for (idx, (arg_name, arg_val)) in function.args_iter(self.context).enumerate() {
+        for (idx, arg) in function.args_iter(self.context).enumerate() {
             let load_arg =
                 uses_stack && (idx >= compiler_constants::NUM_ARG_REGISTERS as usize - 1);
             let arg_reg = if !load_arg {
@@ -428,8 +428,8 @@ impl FuelAsmBuilder<'_, '_> {
                     self.cur_bytecode.push(Op::register_move(
                         arg_copy_reg.clone(),
                         initial_arg_reg,
-                        format!("[fn init: {fn_name}]: copy argument {idx} ({arg_name})"),
-                        self.md_mgr.val_to_span(self.context, *arg_val),
+                        format!("[fn init: {fn_name}]: copy argument {idx} ({})", arg.name),
+                        self.md_mgr.val_to_span(self.context, arg.value),
                     ));
                     arg_copy_reg
                 } else {
@@ -456,20 +456,23 @@ impl FuelAsmBuilder<'_, '_> {
                         VirtualImmediate12::try_new(
                             stack_offset,
                             self.md_mgr
-                                .val_to_span(self.context, *arg_val)
+                                .val_to_span(self.context, arg.value)
                                 .unwrap_or(Span::dummy()),
                         )
                         .expect("Too many arguments, cannot handle."),
                     )),
-                    comment: format!("[fn init: {fn_name}]: load argument {idx} ({arg_name}) from its stack slot"),
-                    owning_span: self.md_mgr.val_to_span(self.context, *arg_val),
+                    comment: format!(
+                        "[fn init: {fn_name}]: load argument {idx} ({}) from its stack slot",
+                        arg.name
+                    ),
+                    owning_span: self.md_mgr.val_to_span(self.context, arg.value),
                 });
 
                 arg_copy_reg
             };
 
             // Remember our arg copy.
-            self.reg_map.insert(*arg_val, arg_reg);
+            self.reg_map.insert(arg.value, arg_reg);
         }
     }
 
@@ -482,7 +485,7 @@ impl FuelAsmBuilder<'_, '_> {
             // A special case for when there's only a single arg, its value (or address) is placed
             // directly in the base register.
             1 => {
-                let (_, val) = function.args_iter(self.context).next().unwrap();
+                let arg = function.args_iter(self.context).next().unwrap();
                 let single_arg_reg = self.reg_seqr.next();
                 match self.program_kind {
                     ProgramKind::Contract => {
@@ -498,7 +501,8 @@ impl FuelAsmBuilder<'_, '_> {
 
                         // The base is an offset.  Dereference it.
                         // XXX val.get_type() should be a pointer if it's not meant to be loaded.
-                        if val
+                        if arg
+                            .value
                             .get_type(self.context)
                             .is_some_and(|t| self.is_copy_type(&t))
                         {
@@ -514,7 +518,7 @@ impl FuelAsmBuilder<'_, '_> {
                         }
                     }
                 }
-                self.reg_map.insert(*val, single_arg_reg);
+                self.reg_map.insert(arg.value, single_arg_reg);
                 Ok(())
             }
 
@@ -533,12 +537,13 @@ impl FuelAsmBuilder<'_, '_> {
                 // Successively load each argument. The asm generated depends on the arg type size
                 // and whether the offset fits in a 12-bit immediate.
                 let mut arg_word_offset = 0;
-                for (name, val) in function.args_iter(self.context) {
+                for arg in function.args_iter(self.context) {
                     let current_arg_reg = self.reg_seqr.next();
 
                     // The function arg type might be a pointer, but the value in the struct will
                     // be of the pointed to type.  So strip the pointer if necessary.
-                    let arg_type = val
+                    let arg_type = arg
+                        .value
                         .get_type(self.context)
                         .map(|ty| ty.get_pointee_type(self.context).unwrap_or(ty))
                         .unwrap();
@@ -552,7 +557,7 @@ impl FuelAsmBuilder<'_, '_> {
                                     args_base_reg.clone(),
                                     offs_reg.clone(),
                                 )),
-                                comment: format!("get offset of argument {name}"),
+                                comment: format!("get offset of argument {}", arg.name),
                                 owning_span: None,
                             });
 
@@ -563,7 +568,7 @@ impl FuelAsmBuilder<'_, '_> {
                                         offs_reg,
                                         VirtualImmediate12::new(0),
                                     )),
-                                    comment: format!("get argument {name}"),
+                                    comment: format!("get argument {}", arg.name),
                                     owning_span: None,
                                 });
                             } else {
@@ -573,7 +578,7 @@ impl FuelAsmBuilder<'_, '_> {
                                         offs_reg,
                                         VirtualImmediate12::new(0),
                                     )),
-                                    comment: format!("get argument {name}"),
+                                    comment: format!("get argument {}", arg.name),
                                     owning_span: None,
                                 });
                             }
@@ -584,7 +589,7 @@ impl FuelAsmBuilder<'_, '_> {
                                     args_base_reg.clone(),
                                     VirtualImmediate12::new(arg_word_offset * 8),
                                 )),
-                                comment: format!("get argument {name}"),
+                                comment: format!("get argument {}", arg.name),
                                 owning_span: None,
                             });
                         } else {
@@ -594,7 +599,7 @@ impl FuelAsmBuilder<'_, '_> {
                                     args_base_reg.clone(),
                                     VirtualImmediate12::new(arg_word_offset),
                                 )),
-                                comment: format!("get argument {name}"),
+                                comment: format!("get argument {}", arg.name),
                                 owning_span: None,
                             });
                         }
@@ -603,13 +608,13 @@ impl FuelAsmBuilder<'_, '_> {
                             arg_word_offset * 8,
                             current_arg_reg.clone(),
                             Some(&args_base_reg),
-                            format!("get offset of argument {name}"),
+                            format!("get offset of argument {}", arg.name),
                             None,
                         );
                     }
 
                     arg_word_offset += arg_type_size.in_words();
-                    self.reg_map.insert(*val, current_arg_reg);
+                    self.reg_map.insert(arg.value, current_arg_reg);
                 }
 
                 Ok(())

sway-core/src/ir_generation.rs

@@ -472,11 +472,11 @@ fn type_correction(ctx: &mut Context) -> Result<(), IrError> {
                 match &instr.get_instruction(ctx).unwrap().op {
                     InstOp::Call(callee, actual_params) => {
                         let formal_params: Vec<_> = callee.args_iter(ctx).collect();
-                        for (param_idx, (actual_param, (_, formal_param))) in
+                        for (param_idx, (actual_param, arg)) in
                             actual_params.iter().zip(formal_params.iter()).enumerate()
                         {
                             let actual_ty = actual_param.get_type(ctx).unwrap();
-                            let formal_ty = formal_param.get_type(ctx).unwrap();
+                            let formal_ty = arg.value.get_type(ctx).unwrap();
                             if actual_ty != formal_ty {
                                 instrs_to_fix.push(TypeCorrection {
                                     actual_ty,

sway-core/src/ir_generation/compile.rs

@@ -1035,8 +1035,10 @@ fn compile_fn(
                     ref_mut_args.insert(param.name.as_str().to_owned());
                 }
                 (
+                    // TODO: We can improve here. Setting all arguments as mutable for now.
+                    IrMutability::Mutable,
                     // Convert the name.
-                    param.name.as_str().into(),
+                    param.name.to_string(),
                     // Convert the type further to a pointer if it's a reference.
                     if param.is_reference {
                         Type::new_typed_pointer(context, ty)
@@ -1054,6 +1056,7 @@ fn compile_fn(
     let keyed_decl = KeyedTyFunctionDecl::new(ast_fn_decl, engines);
     if context.backtrace != Backtrace::None && panicking_fn_cache.can_panic(&keyed_decl, engines) {
         args.push((
+            IrMutability::Immutable,
             FnCompiler::BACKTRACE_FN_ARG_NAME.to_string(),
             Type::new_uint(context, 64),
             None,

sway-core/src/ir_generation/convert.rs

@@ -176,7 +176,7 @@ pub(super) fn convert_resolved_type_info(
                 elem_type.type_id,
                 span,
             )?;
-            Type::new_array(context, elem_type, len as u64)
+            Type::new_array(context, elem_type, len)
         }
 
         TypeInfo::Tuple(fields) => {

sway-core/src/ir_generation/function.rs

@@ -217,11 +217,8 @@ impl<'a> FnCompiler<'a> {
         panicking_fn_cache: &'a mut PanickingFunctionCache,
         compiled_fn_cache: &'a mut CompiledFunctionCache,
     ) -> Self {
-        let lexical_map = LexicalMap::from_iter(
-            function
-                .args_iter(context)
-                .map(|(name, _value)| name.clone()),
-        );
+        let lexical_map =
+            LexicalMap::from_iter(function.args_iter(context).map(|arg| arg.name.clone()));
         FnCompiler {
             engines,
             module,
@@ -267,25 +264,26 @@ impl<'a> FnCompiler<'a> {
         // Function arguments, like all locals need to be in memory, so that their addresses
         // can be taken. So we create locals for each argument and store the value there.
         let entry = self.function.get_entry_block(context);
-        for (arg_name, arg_value) in self
+        for arg in self
             .function
             .args_iter(context)
             .cloned()
             .collect::<Vec<_>>()
         {
-            let local_name = self.lexical_map.insert(arg_name.as_str().to_owned());
+            let local_name = self.lexical_map.insert(arg.name.as_str().to_owned());
             let local_var = self.function.new_unique_local_var(
                 context,
                 local_name.clone(),
-                arg_value.get_type(context).unwrap(),
+                arg.value.get_type(context).unwrap(),
                 None,
+                // TODO We should consider if this is mutable or not here.
                 false,
             );
-            if self.ref_mut_args.contains(&arg_name) {
+            if self.ref_mut_args.contains(&arg.name) {
                 self.ref_mut_args.insert(local_name);
             }
             let local_val = entry.append(context).get_local(local_var);
-            entry.append(context).store(local_val, arg_value);
+            entry.append(context).store(local_val, arg.value);
         }
         match self.compile_code_block(context, md_mgr, ast_block)?.value {
             // Final value must always be in a register, not in memory.
@@ -4735,26 +4733,42 @@ impl<'a> FnCompiler<'a> {
                     .expect("All local symbols must be in the lexical symbol map.");
 
                 // First look for a local variable with the required name
-                let lhs_val = self
-                    .function
-                    .get_local_var(context, name)
-                    .map(|var| {
-                        self.current_block
-                            .append(context)
-                            .get_local(var)
-                            .add_metadatum(context, span_md_idx)
-                    })
-                    .or_else(||
-                        // Now look for an argument with the required name
-                        self.function
-                            .args_iter(context)
-                            .find_map(|(arg_name, arg_val)| (arg_name == name).then_some(*arg_val)))
-                    .ok_or_else(|| {
-                        CompileError::InternalOwned(
+                let local_var = self.function.get_local_var(context, name);
+                let lhs_val = if let Some(local_var) = local_var {
+                    // TODO This check breaks some tests
+                    // if local_var.is_mutable(context) {
+                    self.current_block
+                        .append(context)
+                        .get_local(local_var)
+                        .add_metadatum(context, span_md_idx)
+                    // } else {
+                    //     return Err(CompileError::InternalOwned(
+                    //         format!("Local var `{name}` is not mutable."),
+                    //         base_name.span(),
+                    //     ));
+                    // }
+                } else {
+                    // Not a local var, check is an argument
+                    let Some(arg) = self
+                        .function
+                        .args_iter(context)
+                        .find_map(|arg| (&arg.name == name).then_some(arg.value))
+                    else {
+                        return Err(CompileError::InternalOwned(
                             format!("Variable not found: {name}."),
                             base_name.span(),
-                        )
-                    })?;
+                        ));
+                    };
+
+                    if arg.get_argument(context).unwrap().is_immutable {
+                        return Err(CompileError::InternalOwned(
+                            format!("Func arg `{name}` is not mutable."),
+                            base_name.span(),
+                        ));
+                    } else {
+                        arg
+                    }
+                };
 
                 if indices.is_empty() {
                     if self.ref_mut_args.contains(name) {

sway-ir/src/block.rs

@@ -420,11 +420,19 @@ impl Block {
     ///
     /// For every instruction within the block, any reference to `old_val` is replaced with
     /// `new_val`.
-    pub fn replace_values(&self, context: &mut Context, replace_map: &FxHashMap<Value, Value>) {
+    pub fn replace_values(
+        &self,
+        context: &mut Context,
+        replace_map: &FxHashMap<Value, Value>,
+    ) -> bool {
+        let mut modified = false;
+
         for ins_idx in 0..context.blocks[self.0].instructions.len() {
             let ins = context.blocks[self.0].instructions[ins_idx];
-            ins.replace_instruction_values(context, replace_map);
+            modified |= ins.replace_instruction_values(context, replace_map);
         }
+
+        modified
     }
 
     /// Remove an instruction from this block.
@@ -433,10 +441,13 @@ impl Block {
     /// extra checks should probably be performed here to avoid corruption! Ideally we use get a
     /// user/uses system implemented.  Using `Vec::remove()` is also O(n) which we may want to
     /// avoid someday.
-    pub fn remove_instruction(&self, context: &mut Context, instr_val: Value) {
+    pub fn remove_instruction(&self, context: &mut Context, instr_val: Value) -> bool {
         let ins = &mut context.blocks[self.0].instructions;
         if let Some(pos) = ins.iter().position(|iv| *iv == instr_val) {
             ins.remove(pos);
+            true
+        } else {
+            false
         }
     }
 
@@ -458,9 +469,18 @@ impl Block {
     }
 
     /// Remove instructions from block that satisfy a given predicate.
-    pub fn remove_instructions<T: Fn(Value) -> bool>(&self, context: &mut Context, pred: T) {
+    pub fn remove_instructions<T: Fn(Value) -> bool>(
+        &self,
+        context: &mut Context,
+        pred: T,
+    ) -> bool {
         let ins = &mut context.blocks[self.0].instructions;
+
+        let len_before = ins.len();
         ins.retain(|value| !pred(*value));
+        let len_after = ins.len();
+
+        len_before != len_after
     }
 
     /// Clear the current instruction list and take the one provided.