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

feat: add parametrized labels to LLL #2598

Merged

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Jan 8, 2022

What I did

change labels in LLL to be parameterized. previously, stack items were implicitly consumed from jumpdests using pass, liveness analysis for stack items is impossible in the general case. now, labels in LLL are very similar to subroutines since they take parameters and are properly terminated. fixes #2511.

this also enables translation to certain LLVM backends since implicit stack items cannot be handled in translation to a register machine.

I also added an exit_to instruction for LLL. This compiles the same as goto -- to a JUMP instruction -- but has different scoping semantics. exit_to terminates a subroutine while goto expects control flow to be returned. this is an important hint for liveness analysis, if you hit an exit_to, all stack items in scope must be consumed, whereas if you hit a goto the same requirement does not hold.

How I did it

change the codegen everywhere that stack items are consumed implicitly. in practice, this happens in private functions, and in function common cleanup blocks.

How to verify it

see that tests pass. the assembly output for all contracts i looked at were byte-for-byte the same except for those which return straight out of a loop (using the LLL cleanup_repeat instruction), so i added a couple more tests to test returns out of loops (in the void return and external cases)

Commit message

Change labels in LLL to be parameterized. Previously, stack items were
implicitly consumed from jumpdests using `pass`, which made liveness
analysis for stack items impossible in general. Since labels now take
parameters, they are very similar to subroutines so we could use that
terminology interchangeably.
 
This change also enables translation to other architectures such as LLVM
backends since implicit stack items cannot be handled in translation to
a register machine.
 
Lastly, this commit also added an `exit_to` instruction for LLL. This
compiles the same as `goto` -- to a JUMP instruction -- but has
different scoping semantics. `exit_to` terminates a subroutine while
`goto` expects control flow to be returned. This is an important hint
for liveness analysis. If you hit an `exit_to`, all stack items in scope
must be consumed, whereas if you hit a `goto` the same requirement does
not hold.
 
In order to accomodate the semantic change, this commit changes the
codegen everywhere that stack items are consumed implicitly. In
practice, this happens in private functions, and in function common
cleanup blocks.

Note: the assembly output for all contracts I looked at were
byte-for-byte the same with the exception of those which returned
straight out of a loop (using the LLL `cleanup_repeat` instruction), so
this commit adds a couple more tests to test returns out of loops (in
the void return and external cases)

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@charles-cooper charles-cooper force-pushed the lll_parametrized_labels branch 2 times, most recently from 387eb4d to 0680464 Compare January 8, 2022 22:34
@charles-cooper charles-cooper marked this pull request as draft January 8, 2022 22:46
@charles-cooper charles-cooper force-pushed the lll_parametrized_labels branch 3 times, most recently from ec3ccaa to cc635a5 Compare January 20, 2022 18:33
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jan 22, 2022

This pull request introduces 1 alert when merging 2ea434d into 0004e0c - view on LGTM.com

new alerts:

  • 1 for Unused import

get_comb_opcodes originally stood for "get combined [pseudo- and evm-]
opcodes". what it really meant is get all LLL opcodes.
this commit forces stack items passed to labels to be explicitly named.
this will help liveness analysis of stack items. note that it changes
the structure of labels - instead of allowing labels to occur
arbitrarily in code, they turn into blocks, enforcing scope of the named
stack items.
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 16, 2022

This pull request introduces 1 alert when merging 5d177b2 into e480e91 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 18, 2022

This pull request introduces 1 alert when merging 685696a into e480e91 - view on LGTM.com

new alerts:

  • 1 for Unused import

@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2022

Codecov Report

Merging #2598 (64bc1c5) into master (e480e91) will increase coverage by 0.02%.
The diff coverage is 94.36%.

❗ Current head 64bc1c5 differs from pull request most recent head 1b1ec32. Consider uploading reports for the commit 1b1ec32 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2598      +/-   ##
==========================================
+ Coverage   86.26%   86.29%   +0.02%     
==========================================
  Files          91       91              
  Lines        9646     9739      +93     
  Branches     2437     2466      +29     
==========================================
+ Hits         8321     8404      +83     
- Misses        828      833       +5     
- Partials      497      502       +5     
Impacted Files Coverage Δ
vyper/codegen/module.py 91.15% <ø> (ø)
vyper/utils.py 87.14% <ø> (ø)
vyper/codegen/lll_node.py 92.74% <76.47%> (-1.28%) ⬇️
vyper/lll/compile_lll.py 93.42% <92.72%> (-0.20%) ⬇️
.../codegen/function_definitions/external_function.py 98.09% <100.00%> (+0.13%) ⬆️
.../codegen/function_definitions/internal_function.py 100.00% <100.00%> (ø)
vyper/codegen/return_.py 94.11% <100.00%> (+0.36%) ⬆️
vyper/codegen/self_call.py 95.55% <100.00%> (+0.81%) ⬆️
vyper/codegen/stmt.py 88.62% <100.00%> (-0.36%) ⬇️
vyper/compiler/output.py 93.82% <100.00%> (+0.17%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e480e91...1b1ec32. Read the comment docs.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 23, 2022

This pull request introduces 1 alert when merging 93fe033 into 14cbedb - view on LGTM.com

new alerts:

  • 1 for Unused local variable

cases:
- internal void
- internal with return
- external void
- external with return
@charles-cooper charles-cooper changed the title add parametrized labels to LLL feat: add parametrized labels to LLL Feb 23, 2022
Change labels in LLL to be parameterized. Previously, stack items were
implicitly consumed from jumpdests using `pass`, which made liveness
analysis for stack items impossible in general. Since labels now take
parameters, they are very similar to subroutines so we could use that
terminology interchangeably.

This change also enables translation to other architectures such as LLVM
backends since implicit stack items cannot be handled in translation to
a register machine.

Lastly, this commit also added an `exit_to` instruction for LLL. This
compiles the same as `goto` -- to a JUMP instruction -- but has
different scoping semantics. `exit_to` terminates a subroutine while
`goto` expects control flow to be returned. This is an important hint
for liveness analysis. If you hit an `exit_to`, all stack items in scope
must be consumed, whereas if you hit a `goto` the same requirement does
not hold.

In order to accomodate the semantic change, this commit changes the
codegen everywhere that stack items are consumed implicitly. In
practice, this happens in private functions, and in function common
cleanup blocks.

Note: the assembly output for all contracts I looked at were
byte-for-byte the same with the exception of those which returned
straight out of a loop (using the LLL `cleanup_repeat` instruction), so
this commit adds a couple more tests to test returns out of loops (in
the void return and external cases)
@charles-cooper charles-cooper marked this pull request as ready for review February 23, 2022 01:44
vyper/compiler/output.py Outdated Show resolved Hide resolved
@charles-cooper charles-cooper enabled auto-merge (squash) February 23, 2022 03:45
@charles-cooper charles-cooper merged commit 3de52b3 into vyperlang:master Feb 23, 2022
@charles-cooper charles-cooper deleted the lll_parametrized_labels branch February 23, 2022 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLL: add parameter/scoping to labels
3 participants