forked from vyperlang/vyper
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Dedup IRnodes takehome! #1
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ang#3969) Show entire endAuction method
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>
This reverts commit 086558b.
in case of copies, the ID will be copied appropriately
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first approach I tried:
Attempt to identify all complex ir nodes by recursing through the whole ir tree.
Add these IR nodes to a list. Then use the multi_scope utility to try to eliminate
any duplicates all at once. I created an "inline_complex" method that does this.
The problem with this approach is that _optimize checks for duplicated symbols.
Unfortunately, to determine whether a node is complex, we have to call arg._optimized.is_complex_ir. This then fails since we have duplicated symbols (which we're attempting to get rid of). So it's a bit of a chicken and egg problem.
Also, we have to call the inline_complex method before optimize (again because of
the issue with optimize checking for duplicated symbols). We cannot call inline_complex
from within optimize because that would cause mutual recursion.
From here I see two potential approaches:
The second approach I tried is the approach that I am taking in this PR.
I create a class-level cache in IRnode that contains a mapping of the ids of IRnodes that we'd like to cache to the variable representing the IRnode.
Then, when constructing a new IRnode, check whether each argument's id is in the cache already. If it is, use the argument's unique id to look up the variable that corresponds to the IRnode, and replace the argument with it.
There are some cases where we don't want to attempt to cache the argument though, namely if the argument should be inlined, if the valency is not 1, or if arg.value is equal to "pass".
If the IRnode argument should be cached but isn't yet, we construct a new variable representing the IRnode and store it in the cache. We also add the new variable and the corresponding IRnode to a list so that we can properly define the variable in a "with" expression later.
Finally, I replace the current IRnode's args with the new, cached arguments. Additionally, I wrap the current IRnode in a "with" which defines the new variable that points to each cached arguments.
I think I currently have an issue with some cycles in the IRnodes, which is causing infinite recursion in the unique_symbols utility. I think that the way that I am updating the IRnode to include the "with" scoping is a bit sketchy and part of the problem, but I'm not exactly sure how to fix it.
Also, I did remove some cache_when_complex statements for testing purposes (specifically ones in arithmetic.py), but I think I accidentally removed them from the lib/python3.10/site-packages/vyper/ files, not from ones in the vyper repo, so they're not showing up in the PR.