-
Notifications
You must be signed in to change notification settings - Fork 20
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
[EraVM] Add support for splitting live ranges of PHI nodes in loops #703
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.
I appreciate the documentation in the code, it helped me a lot. Still, I can't fully comprehend the patch as logic seems complex to me.
This is mainly used to test this pass by running it with opt. Signed-off-by: Vladimir Radosavljevic <[email protected]>
… of PHI nodes in loops Signed-off-by: Vladimir Radosavljevic <[email protected]>
This is useful for loops with large switch statements where PHI nodes are used frequently, and we want to keep these variables in a single register. Signed-off-by: Vladimir Radosavljevic <[email protected]>
0f61c65
to
28ada23
Compare
Idea to do this optimization came when I was investigating performance drop (caused by regalloc not being able to preserve variable in single register and added unnecessary copy instructions in all opcodes) after one of the changes in the EVMInterpreter. We had following case in one of the opcode in the EVMInterpreter:
After I manually changed this to:
regalloc managed to preserve variable in a single register and removed a lot of copy instructions.
After this optimization:
I will create a ticket for this kind of optimization, since we could benefit from indexed loads and stores (there are optimizations in DAGCombine for this) and we can do something similar in MIR to create more opportunities for post increment loads and stores. Btw, my idea is to disable this optimization by default and to enable it only for EVMInterpreter. I don't think we should do this optimization for other contracts. |
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.
LGTM. Thank you for putting effort into refactoring.
No description provided.