-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Skip returning a value when the type is unit #7401
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the returned unit value is actually used? Please see my remark in this comment.
CodSpeed Performance ReportMerging #7401 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To the returned unit value being used, we had an ICE before in case of asm
blocks that were returning unit, where the return register was not set. The issue was fixed in #6404.
I think a solution to handle this would be:
What do you think? |
Yes makes sense. We are free to define the calling conventions. It is cheaper then analyzing if there are callers that use the returned value. |
Size changes on our `should_pass` tests:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see all those reductions in code size and gas usage in tests 😄 💪
test/src/e2e_vm_tests/test_programs/should_pass/language/unit_ret_use/src/main.sw
Outdated
Show resolved
Hide resolved
test/src/e2e_vm_tests/test_programs/should_pass/language/unit_ret_use/src/main.sw
Outdated
Show resolved
Hide resolved
test/src/e2e_vm_tests/test_programs/should_pass/language/unit_ret_use/src/main.sw
Outdated
Show resolved
Hide resolved
test/src/e2e_vm_tests/test_programs/should_pass/language/unit_ret_use/Forc.toml
Outdated
Show resolved
Hide resolved
test/src/e2e_vm_tests/test_programs/should_pass/language/unit_ret_use/src/main.sw
Outdated
Show resolved
Hide resolved
…into vaivaswatha/unit_ret_skip
#7381 (comment)