Fix memcpyopt to allow multiple passes of optimisations

forc/Cargo.toml

@@ -56,8 +56,6 @@ whoami.workspace = true
 
 [dev-dependencies]
 completest-pty.workspace = true
-rexpect.workspace = true
 
 [lints]
 workspace = true
-

sway-ir/src/optimize/memcpyopt.rs

@@ -80,9 +80,22 @@ fn local_copy_prop_prememcpy(
                     instr_info_map.insert(inst, info());
                 }
             }
+            // Instructions that "write" to symbols
             Instruction {
                 op: InstOp::Store { dst_val_ptr, .. },
                 ..
+            }
+            | Instruction {
+                op: InstOp::MemCopyVal { dst_val_ptr, .. },
+                ..
+            }
+            | Instruction {
+                op: InstOp::MemClearVal { dst_val_ptr },
+                ..
+            }
+            | Instruction {
+                op: InstOp::MemCopyBytes { dst_val_ptr, .. },
+                ..
             } => {
                 if let Some(local) = get_referred_symbol(context, *dst_val_ptr) {
                     stores_map
@@ -151,28 +164,46 @@ fn local_copy_prop_prememcpy(
                     let dst_local_stores = stores_map.get(&dst_local).unwrap_or(&temp_empty1);
                     let src_local_stores = stores_map.get(&src_local).unwrap_or(&temp_empty2);
                     let dst_local_loads = loads_map.get(&dst_local).unwrap_or(&temp_empty3);
-                    // This must be the only store of dst_local.
-                    if dst_local_stores.len() != 1 || dst_local_stores[0] != instr_val
-                        ||
-                        // All stores of src_local must be in the same block, prior to src_load.
-                        !src_local_stores.iter().all(|store_val|{
-                            let instr_info = instr_info_map.get(store_val).unwrap();
-                            let src_load_info = instr_info_map.get(src_load).unwrap();
-                            instr_info.block == block && instr_info.pos < src_load_info.pos
-                        })
-                        ||
-                        // All loads of dst_local must be after this instruction, in the same block.
-                        !dst_local_loads.iter().all(|load_val| {
-                            let instr_info = instr_info_map.get(load_val).unwrap();
-                            instr_info.block == block && instr_info.pos > pos
-                        })
-                        // We don't deal with symbols that escape.
-                        || escaped_symbols.contains(&dst_local)
-                        || escaped_symbols.contains(&src_local)
-                        // We don't deal part copies.
-                        || dst_local.get_type(context) != src_local.get_type(context)
-                        // We don't replace the destination when it's an arg.
-                        || matches!(dst_local, Symbol::Arg(_))
+
+                    // Battery of checks
+                    // We bail the optimization if ANY check is true
+
+                    // Check 1. Fail if the destination symbol has more than one write
+                    let more_than_one_write =
+                        dst_local_stores.len() != 1 || dst_local_stores[0] != instr_val;
+
+                    // Check 2. Fail if any write to src occurs after the src load.
+                    // We need to be sure that the source of the store has the last value
+                    // of the symbol (no writes after the load)
+                    let any_src_write_after_src_load = !src_local_stores.iter().all(|src_store| {
+                        let src_store_info = instr_info_map.get(src_store).unwrap();
+                        let src_load_info = instr_info_map.get(src_load).unwrap();
+                        src_store_info.block == block && src_store_info.pos < src_load_info.pos
+                    });
+
+                    // Check 3. any loads of the destination variable occur prior to this operation in the execution flow.
+                    let any_dst_load_before = !dst_local_loads.iter().all(|load_val| {
+                        let load_val_info = instr_info_map.get(load_val).unwrap();
+                        load_val_info.block == block && load_val_info.pos > pos
+                    });
+
+                    // Check 4. ensure neither symbol escape
+                    let any_escape = escaped_symbols.contains(&dst_local)
+                        || escaped_symbols.contains(&src_local);
+
+                    // Check 5. We don't deal part copies, types must be the same.
+                    let types_different =
+                        dst_local.get_type(context) != src_local.get_type(context);
+
+                    // Check 6. We don't replace the destination when it's an arg.
+                    let is_arg = matches!(dst_local, Symbol::Arg(_));
+
+                    if more_than_one_write
+                        || any_src_write_after_src_load
+                        || any_dst_load_before
+                        || any_escape
+                        || types_different
+                        || is_arg
                     {
                         None
                     } else {
@@ -257,6 +288,7 @@ fn local_copy_prop_prememcpy(
     for block in blocks {
         block.remove_instructions(context, |value| to_delete.contains(&value));
     }
+
     Ok(true)
 }
 

sway-ir/src/pass_manager.rs

@@ -410,18 +410,27 @@ impl PassManager {
             ir.verify()?;
         }
 
-        let mut modified = false;
-        for pass in passes.flatten_pass_group() {
-            let modified_in_pass = self.actually_run(ir, pass)?;
+        let mut global_modified = false;
+
+        for _ in 0..2 {
+            let mut iter_modified = false;
+
+            for pass in passes.flatten_pass_group() {
+                let modified = self.actually_run(ir, pass)?;
+                iter_modified |= modified;
 
-            if print_opts.passes.contains(pass) && (!print_opts.modified_only || modified_in_pass) {
-                print_ir_after_pass(ir, self.lookup_registered_pass(pass).unwrap());
+                if print_opts.passes.contains(pass) && (!print_opts.modified_only || modified) {
+                    print_ir_after_pass(ir, self.lookup_registered_pass(pass).unwrap());
+                }
+
+                if verify_opts.passes.contains(pass) && (!verify_opts.modified_only || modified) {
+                    ir.verify()?;
+                }
             }
 
-            modified |= modified_in_pass;
-            if verify_opts.passes.contains(pass) && (!verify_opts.modified_only || modified_in_pass)
-            {
-                ir.verify()?;
+            global_modified |= iter_modified;
+            if !iter_modified {
+                break;
             }
         }
 
@@ -433,7 +442,7 @@ impl PassManager {
             ir.verify()?;
         }
 
-        Ok(modified)
+        Ok(global_modified)
     }
 
     /// Get reference to a registered pass.

forc/tests/fixtures/test_contract/Forc.toml → test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract/Forc.toml

@@ -5,4 +5,4 @@ license = "Apache-2.0"
 name = "test_contract"
 
 [dependencies]
-std = { path = "../../../../sway-lib-std/" }
+std = { path = "../../../../reduced_std_libs/sway-lib-std-assert" }

test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract/snapshot.toml

@@ -0,0 +1,5 @@
+cmds = [
+    "forc test --logs --path {root}",
+    "forc test --raw-logs --path {root}",
+    "forc test --logs --raw-logs --path {root}",
+]

test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract/stdout.snap

@@ -0,0 +1,72 @@
+---
+source: test/src/snapshot/mod.rs
+---
+> forc test --logs --path test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract
+exit status: 0
+output:
+    Building test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract
+   Compiling library std (test/src/e2e_vm_tests/reduced_std_libs/sway-lib-std-assert)
+   Compiling contract test_contract (test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract)
+    Finished debug [unoptimized + fuel] target(s) [1.04 KB] in ???
+     Running 2 tests, filtered 0 tests
+
+tested -- test_contract
+
+      test test_log_4 ... ok (???, 604 gas)
+           decoded log values:
+4, log rb: 1515152261580153489
+      test test_log_2 ... ok (???, 604 gas)
+           decoded log values:
+2, log rb: 1515152261580153489
+
+test result: OK. 2 passed; 0 failed; finished in ???
+
+    Finished in ???
+
+> forc test --raw-logs --path test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract
+exit status: 0
+output:
+    Building test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract
+   Compiling library std (test/src/e2e_vm_tests/reduced_std_libs/sway-lib-std-assert)
+   Compiling contract test_contract (test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract)
+    Finished debug [unoptimized + fuel] target(s) [1.04 KB] in ???
+     Running 2 tests, filtered 0 tests
+
+tested -- test_contract
+
+      test test_log_4 ... ok (???, 604 gas)
+           raw logs:
+[{"LogData":{"data":"0000000000000004","digest":"8005f02d43fa06e7d0585fb64c961d57e318b27a145c857bcd3a6bdb413ff7fc","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":11288,"ptr":12464,"ra":0,"rb":1515152261580153489}}]
+      test test_log_2 ... ok (???, 604 gas)
+           raw logs:
+[{"LogData":{"data":"0000000000000002","digest":"cd04a4754498e06db5a13c5f371f1f04ff6d2470f24aa9bd886540e5dce77f70","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":11288,"ptr":12464,"ra":0,"rb":1515152261580153489}}]
+
+test result: OK. 2 passed; 0 failed; finished in ???
+
+    Finished in ???
+
+> forc test --logs --raw-logs --path test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract
+exit status: 0
+output:
+    Building test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract
+   Compiling library std (test/src/e2e_vm_tests/reduced_std_libs/sway-lib-std-assert)
+   Compiling contract test_contract (test/src/e2e_vm_tests/test_programs/should_pass/forc/test_contract)
+    Finished debug [unoptimized + fuel] target(s) [1.04 KB] in ???
+     Running 2 tests, filtered 0 tests
+
+tested -- test_contract
+
+      test test_log_4 ... ok (???, 604 gas)
+           decoded log values:
+4, log rb: 1515152261580153489
+           raw logs:
+[{"LogData":{"data":"0000000000000004","digest":"8005f02d43fa06e7d0585fb64c961d57e318b27a145c857bcd3a6bdb413ff7fc","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":11288,"ptr":12464,"ra":0,"rb":1515152261580153489}}]
+      test test_log_2 ... ok (???, 604 gas)
+           decoded log values:
+2, log rb: 1515152261580153489
+           raw logs:
+[{"LogData":{"data":"0000000000000002","digest":"cd04a4754498e06db5a13c5f371f1f04ff6d2470f24aa9bd886540e5dce77f70","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":11288,"ptr":12464,"ra":0,"rb":1515152261580153489}}]
+
+test result: OK. 2 passed; 0 failed; finished in ???
+
+    Finished in ???