From c944550692d9bc862e3441660211e3e29079ca9e Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 10 Jan 2025 11:43:53 +0530 Subject: [PATCH 1/2] [asmgen] pushing / popping registers for call should handle all def'd regs Fixes #6814 --- .../allocated_abstract_instruction_set.rs | 14 ++--- .../pusha_popa_multiple_defreg/Forc.lock | 13 +++++ .../pusha_popa_multiple_defreg/Forc.toml | 8 +++ .../pusha_popa_multiple_defreg/src/main.sw | 56 +++++++++++++++++++ .../pusha_popa_multiple_defreg/test.toml | 3 + 5 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/Forc.lock create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/Forc.toml create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/src/main.sw create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/test.toml diff --git a/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs b/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs index e939ef633a2..69d599e2bee 100644 --- a/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs +++ b/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs @@ -3,7 +3,7 @@ use crate::{ asm_lang::{ allocated_ops::{AllocatedOpcode, AllocatedRegister}, AllocatedAbstractOp, ConstantRegister, ControlFlowOp, Label, RealizedOp, - VirtualImmediate12, VirtualImmediate18, VirtualImmediate24, + VirtualImmediate12, VirtualImmediate18, VirtualImmediate24, VirtualRegister, }, }; @@ -77,21 +77,21 @@ impl AllocatedAbstractInstructionSet { .fold( (IndexMap::new(), IndexSet::new()), |(mut reg_sets, mut active_sets), op| { - let reg = match &op.opcode { + let regs: Box> = match &op.opcode { Either::Right(ControlFlowOp::PushAll(label)) => { active_sets.insert(*label); - None + Box::new(std::iter::empty()) } Either::Right(ControlFlowOp::PopAll(label)) => { active_sets.swap_remove(label); - None + Box::new(std::iter::empty()) } - Either::Left(alloc_op) => alloc_op.def_registers().into_iter().next(), - Either::Right(ctrl_op) => ctrl_op.def_registers().into_iter().next(), + Either::Left(alloc_op) => Box::new(alloc_op.def_registers().into_iter()), + Either::Right(ctrl_op) => Box::new(ctrl_op.def_registers().into_iter()), }; - if let Some(reg) = reg { + for reg in regs { for active_label in active_sets.clone() { reg_sets .entry(active_label) diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/Forc.lock new file mode 100644 index 00000000000..5bfd962ec35 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/Forc.lock @@ -0,0 +1,13 @@ +[[package]] +name = "core" +source = "path+from-root-B299C98691745B40" + +[[package]] +name = "pusha_popa_multiple_defreg" +source = "member" +dependencies = ["std"] + +[[package]] +name = "std" +source = "path+from-root-B299C98691745B40" +dependencies = ["core"] diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/Forc.toml new file mode 100644 index 00000000000..e1407943836 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/Forc.toml @@ -0,0 +1,8 @@ +[project] +authors = ["Fuel Labs "] +entry = "main.sw" +license = "Apache-2.0" +name = "pusha_popa_multiple_defreg" + +[dependencies] +std = { path = "../../../../reduced_std_libs/sway-lib-std-assert" } diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/src/main.sw new file mode 100644 index 00000000000..6c6b17340d7 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/src/main.sw @@ -0,0 +1,56 @@ +contract; + +abi IncorrectPushaPopa { + #[storage(read)] + fn incorrect_pusha_popa() -> (); +} + +impl IncorrectPushaPopa for Contract { + #[storage(read)] + fn incorrect_pusha_popa() -> () { + setup(); + () + } +} + +#[storage(read)] +fn setup() -> () { + let a: u64 = 1; + let b: u64 = 1; + let c: u64 = 1; + //call a few times to avoid inline + store_read(); + let r = asm(r, a: a, b: b, c: c, d: store_read()) { + movi r i0; + add r a b; // r = a + b = 2 + add r r c; // r = a + b + c = 3 c value is overwritten by store_read, so we get 2 instead + add r r d; // r = a + b + c + d = 3 d returns 0 + r + }; + assert(r == 3); + () +} + +#[storage(read)] +fn store_read() -> u64 { + let a = asm(slot, a, b, c) { + movi c i32; + aloc c; + move slot hp; + srw a b slot; // somehow make b allocate to $r3 + movi c i0; + add a a slot; + sub a a slot; + add a a b; + add a a c; + a + }; + a - a // return 0 and make sure a is not dced +} + +#[test] +fn incorrect_pusha_popa() -> () { + let c = abi(IncorrectPushaPopa, CONTRACT_ID); + c.incorrect_pusha_popa(); + () +} diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/test.toml b/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/test.toml new file mode 100644 index 00000000000..ad1782559e7 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/pusha_popa_multiple_defreg/test.toml @@ -0,0 +1,3 @@ +category = "unit_tests_pass" +validate_abi = false +expected_warnings = 0 From f91f13654b6cd3574b6451158b7761e8988ae369 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 10 Jan 2025 12:20:57 +0530 Subject: [PATCH 2/2] fix warnings --- .../asm_generation/fuel/allocated_abstract_instruction_set.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs b/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs index 69d599e2bee..d887a682d76 100644 --- a/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs +++ b/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs @@ -3,7 +3,7 @@ use crate::{ asm_lang::{ allocated_ops::{AllocatedOpcode, AllocatedRegister}, AllocatedAbstractOp, ConstantRegister, ControlFlowOp, Label, RealizedOp, - VirtualImmediate12, VirtualImmediate18, VirtualImmediate24, VirtualRegister, + VirtualImmediate12, VirtualImmediate18, VirtualImmediate24, }, };