Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug][compiler-v2] Optimized stackless bytecode may not always yield optimized file format bytecode #12404

Open
vineethk opened this issue Mar 6, 2024 · 4 comments
Assignees
Labels
bug Something isn't working compiler-v2-stable compiler-v2 stale-exempt Prevents issues from being automatically marked and closed as stale

Comments

@vineethk
Copy link
Contributor

vineethk commented Mar 6, 2024

🐛 Bug

Our stackless bytecode optimization pipeline is geared towards optimizing the stackless bytecode (i.e., create fewer stackless bytecode instructions and fewer locals used by the instructions). This may not always result in fewer file-format bytecode: we need to investigate whether this is inevitable, or whether we can get some assurances about the translation from stackless bytecode to file format bytecode.

To reproduce

Run aptos-move/aptos-transactional-test-harness/tests/v2-tests/default_int_size.move with and without the default optimization pipeline, observe the stackless bytecode and file format bytecode generated (using MVC_LOG=debug).

With optimization, we produce fewer stackless bytecode instructions and fewer locals, but slightly increase the file format bytecode instructions (and decrease locals).

@vineethk vineethk added bug Something isn't working compiler-v2 labels Mar 6, 2024
@vineethk vineethk moved this from 🆕 New to 📋 Backlog in Move Language and Runtime Mar 6, 2024
@vineethk vineethk moved this from 📋 Backlog to 🆕 New in Move Language and Runtime Mar 6, 2024
@wrwg wrwg moved this from 🆕 New to 📋 Backlog in Move Language and Runtime Mar 7, 2024
@sausagee sausagee added the stale-exempt Prevents issues from being automatically marked and closed as stale label Mar 8, 2024
@aptos-labs aptos-labs deleted a comment from Gft654 Mar 18, 2024
@vineethk
Copy link
Contributor Author

A detailed example of this issue

Consider the following code (we will focus on the test function in this writeup, but I have given the full code for completeness):

module 0xc0ffee::m {
    fun test1(x: u64): u64 {
        x + 1
    }

    fun test2(x: u64): u64 {
        x + 2
    }

    public fun test(): u64 {
        test1(2) + test2(5)
    }
}

With variable coalescing (and related optimizations) on, we produce the following stackless bytecode:

public fun m::test(): u64 {
     var $t0: u64 [unused]
     var $t1: u64 [unused]
     var $t2: u64
     var $t3: u64 [unused]
     var $t4: u64
  0: $t2 := 2
  1: $t2 := m::test1($t2)
  2: $t4 := 5
  3: $t4 := m::test2($t4)
  4: $t2 := +($t2, $t4)
  5: return $t2
}

The above (optimized) stackless bytecode yields the following file format bytecode:

public test(): u64 /* def_idx: 0 */ {
L0:	loc0: u64
L1:	loc1: u64
B0:
	0: LdU64(2)
	1: StLoc[0](loc0: u64)
	2: CopyLoc[0](loc0: u64)
	3: Call test1(u64): u64
	4: LdU64(5)
	5: StLoc[1](loc1: u64)
	6: CopyLoc[1](loc1: u64)
	7: Call test2(u64): u64
	8: StLoc[1](loc1: u64)
	9: StLoc[0](loc0: u64)
	10: CopyLoc[0](loc0: u64)
	11: MoveLoc[1](loc1: u64)
	12: Add
	13: Ret
}

Now, consider the case where we do not use variable coalescing (and related optimizations). It produces the following stackless bytecode, which uses more variables than the optimized version above:

public fun m::test(): u64 {
     var $t0: u64
     var $t1: u64
     var $t2: u64
     var $t3: u64
     var $t4: u64
  0: $t2 := 2
  1: $t1 := m::test1($t2)
  2: $t4 := 5
  3: $t3 := m::test2($t4)
  4: $t0 := +($t1, $t3)
  5: return $t0
}

However, this unoptimized stackless bytecode produces better file format bytecode! See below:

public test(): u64 /* def_idx: 0 */ {
B0:
        0: LdU64(2)
        1: Call test1(u64): u64
        2: LdU64(5)
        3: Call test2(u64): u64
        4: Add
        5: Ret
}

@vineethk
Copy link
Contributor Author

@brmataptos
Copy link
Contributor

Eliminating this pattern for each local that is dead after the next operation will solve this case:

	1: StLoc[0](loc0: u64)
	2: CopyLoc[0](loc0: u64)

@brmataptos
Copy link
Contributor

A few more tests cases are provided by the move-cli tests when compiled with v2, they show marked increase in number of bytes:

diff ./third_party/move/tools/move-cli/tests/sandbox_tests/build_modules_and_scripts/args.exp ./third_party/move/tools/move-cli/tests/sandbox_tests/build_modules_and_scripts/args.v2_exp
< Wrote 88 bytes of module ID's and code
> Wrote 129 bytes of module ID's and code
diff ./third_party/move/tools/move-cli/tests/sandbox_tests/cyclic_multi_module_publish/args.exp ./third_party/move/tools/move-cli/tests/sandbox_tests/cyclic_multi_module_publish/args.v2_exp
< Wrote 239 bytes of module ID's and code
> Wrote 321 bytes of module ID's and code
diff ./third_party/move/tools/move-cli/tests/sandbox_tests/examples_published_dev_mode/args.exp ./third_party/move/tools/move-cli/tests/sandbox_tests/examples_published_dev_mode/args.v2_exp
< Wrote 98 bytes of module ID's and code
> Wrote 139 bytes of module ID's and code
< Wrote 198 bytes of module ID's and code
> Wrote 280 bytes of module ID's and code
diff ./third_party/move/tools/move-cli/tests/sandbox_tests/module_disassemble/args.exp ./third_party/move/tools/move-cli/tests/sandbox_tests/module_disassemble/args.v2_exp
< Wrote 544 bytes of module ID's and code
> Wrote 628 bytes of module ID's and code
diff ./third_party/move/tools/move-cli/tests/sandbox_tests/module_publish_view/args.exp ./third_party/move/tools/move-cli/tests/sandbox_tests/module_publish_view/args.v2_exp
< Wrote 152 bytes of module ID's and code
> Wrote 193 bytes of module ID's and code
diff ./third_party/move/tools/move-cli/tests/sandbox_tests/multi_module_publish/args.exp ./third_party/move/tools/move-cli/tests/sandbox_tests/multi_module_publish/args.v2_exp
< Wrote 250 bytes of module ID's and code
> Wrote 332 bytes of module ID's and code
diff ./third_party/move/tools/move-cli/tests/sandbox_tests/republish/args.exp ./third_party/move/tools/move-cli/tests/sandbox_tests/republish/args.v2_exp
< Wrote 176 bytes of module ID's and code
> Wrote 258 bytes of module ID's and code
< Wrote 176 bytes of module ID's and code
> Wrote 258 bytes of module ID's and code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler-v2-stable compiler-v2 stale-exempt Prevents issues from being automatically marked and closed as stale
Projects
Status: 📋 Backlog
Development

No branches or pull requests

4 participants