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[venom]: implement mem2var and sccp passes #3941

Merged
merged 563 commits into from
Apr 26, 2024

Conversation

harkal
Copy link
Collaborator

@harkal harkal commented Apr 12, 2024

What I did

Implemented two additional Venom optimization passes: Mem2Var and SCCP.

How I did it

How to verify it

Commit message

This commit adds two additional optimization passes to the Venom
pipeline, `mem2var` and `Sparse Conditional Constant Propagation`.

The `mem2var` pass has the purpose of promoting some of the memory
accesses that the frontend emits to Venom variables, optimizing out
memory reads and writes. It is analogous to the `mem2reg` pass in LLVM.
Right now it only applies promotions conservatively, but is the basis
for more advanced memory optimizations in the future.

To facilitate the implementation of `mem2var` this commit additionally
modifies the original IR emitter to emit "abstract" memory locations
(i.e. "allocas") instead of hard-coded pointers if the venom pipeline is
enabled. A small amount of refactoring was done to the memory allocator
to enable this switch to be implemented cleanly.

The `sccp` pass is responsible for evaluating and propagating constants
in the code, eliminating conditional branches and performing dead code
elimination in the process. It is a linear `O(2n)` pass, based on the
classical algorithm by Wegman and Zadeck.

References:
https://dl.acm.org/doi/pdf/10.1145/103135.103136

Description for the changelog

Cute Animal Picture

image

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good, just a few small comments. might hold off on merging until 0.4.1

Comment on lines 454 to 461

if experimental_codegen:
with pytest.raises(StaticAssertionException):
get_contract(code)
else:
c = get_contract(code)
with tx_failed():
c.foo()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should change the code as above so the runtime check is appropriately checked

Comment on lines 77 to 92
def test_exponent_negative_power(get_contract, tx_failed, base, experimental_codegen):
# #2985
code = f"""
@external
def bar() -> int16:
x: int16 = -2
return {base} ** x
"""
c = get_contract(code)
# known bug: 2985
with tx_failed():
c.bar()
if experimental_codegen:
with pytest.raises(StaticAssertionException):
get_contract(code)
else:
c = get_contract(code)
# known bug: 2985
with tx_failed():
c.bar()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

typ: VyperType,
is_internal: bool,
is_mutable: bool = True,
internal_function=False,
Copy link
Member

@charles-cooper charles-cooper Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added this, but i'm not a huge fan of this. it's fine for now though

@@ -79,4 +86,9 @@ def _collapse_chained_blocks(self, entry: IRBasicBlock):
def _run_pass(self, ctx: IRFunction, entry: IRBasicBlock) -> None:
self.ctx = ctx

self._collapse_chained_blocks(entry)
for _ in range(len(ctx.basic_blocks)): # essentialy `while True`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for _ in range(len(ctx.basic_blocks)): # essentialy `while True`
for _ in range(len(ctx.basic_blocks)): # essentially `while True`

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just a couple small things and then we can merge!

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@charles-cooper charles-cooper enabled auto-merge (squash) April 26, 2024 13:36
@charles-cooper charles-cooper merged commit 7d28a50 into vyperlang:master Apr 26, 2024
151 checks passed
@charles-cooper charles-cooper deleted the feature/sccp branch April 26, 2024 13:51
@charles-cooper charles-cooper restored the feature/sccp branch April 26, 2024 13:51
electriclilies pushed a commit to electriclilies/vyper that referenced this pull request Apr 27, 2024
This commit adds two additional optimization passes to the Venom
pipeline, `mem2var` and `Sparse Conditional Constant Propagation`.

The `mem2var` pass has the purpose of promoting some of the memory
accesses that the frontend emits to Venom variables, optimizing out
memory reads and writes. It is analogous to the `mem2reg` pass in LLVM.
Right now it only applies promotions conservatively, but is the basis
for more advanced memory optimizations in the future.

To facilitate the implementation of `mem2var` this commit additionally
modifies the original IR emitter to emit "abstract" memory locations
(i.e. "allocas") instead of hard-coded pointers if the venom pipeline is
enabled. A small amount of refactoring was done to the memory allocator
to enable this switch to be implemented cleanly.

The `sccp` pass is responsible for evaluating and propagating constants
in the code, eliminating conditional branches and performing dead code
elimination in the process. It is a linear `O(2n)` pass, based on the
classical algorithm by Wegman and Zadeck.

References:
https://dl.acm.org/doi/pdf/10.1145/103135.103136

---------

Co-authored-by: Charles Cooper <cooper.charles.m@gmail.com>
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.

None yet

4 participants