SSA: Refactor shuffler helper and add a case to prefer swap if still possible instead of shrinking first - #16515
Merged
Merged
Conversation
clonker
previously approved these changes
Mar 18, 2026
clonker
left a comment
clonker
left a comment
Member
There was a problem hiding this comment.
Just two tiny stylistic points, beyond that good change! Makes the code easier to reason about, too.
| { | ||
| auto const & top = m_stackData.back(); | ||
| yulAssert(_offset.value < size()); | ||
| auto const & slot = m_stackData[_offset.value]; |
Member
There was a problem hiding this comment.
Maybe StackData should get these accessors with offset and depth, too :) not in this PR in any case.
Contributor
Author
There was a problem hiding this comment.
It could be helpful :)
Previously, if a slot is desirable to be dupped, but is out of reach, we would try to shrink the stack and then bringing it to reachable region. However, even if it is out-of-reach for dup, it might still be in reach for swap, so if it is safe to swap, we can do it directly.
blishko
force-pushed
the
ssa-cfg-shuffler-updates
branch
from
March 18, 2026 13:51
5fb846c to
8a66e9f
Compare
Contributor
Author
|
Style should be fixed now :) |
clonker
approved these changes
Mar 19, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR consists of two changes in two separate commits. I recommend to review the commits separately.
First one is proposal to refactor shufller's helper
dupDeepSlotIfRequired.It attempts to make the code more compact to facilitate reading and understanding this method.
It also extracts a check to a helper function on
State. This helper is then reused in the second commit where we attempt to check if a slot is swap-reachable even if it is not dup-reachable. If is swap reachable and the swap is safe to do, we do it, instead of first attempting to shrink the stack (in order to bring this slot into dup-reachable space).On
deep_argument_shrinktest case this produces locally better bytecode.It would be good to run a larger evaluation on larger contracts to see what is the effect of this change.