Skip to content

SSA CFG: Fix shuffler cycle caused by overeager pop of spilled top - #16929

Merged
clonker merged 2 commits into
developfrom
fix-ssa-cfg-shuffler-cycle-pop-spilled-top
Aug 20, 2026
Merged

SSA CFG: Fix shuffler cycle caused by overeager pop of spilled top#16929
clonker merged 2 commits into
developfrom
fix-ssa-cfg-shuffler-cycle-pop-spilled-top

Conversation

@clonker

@clonker clonker commented Aug 19, 2026

Copy link
Copy Markdown
Member
  • shrinkStack eagerly popped any spilled stack top before trying the swap logic below.
    When the spilled value was still demanded at an unfilled arg position, the pop just re-opened the demand, fixArgsSlot reloaded the same value, and the next shrink popped it again.
  • Fix: the eager pop now only fires for junk and dead slots; a spilled top falls through to the swap logic, which places it directly into a demanded position (which the reload-side depth guards guarantee is swap-reachable). Since the reload only fires while an unfilled demand exists, and shrink swaps instead of popping in exactly that state, the pop and the reload can no longer alternate on the same state.
  • Popping a spilled top stays possible, it's just demoted into the shrinkPriority cascade: canBeFreelyGenerated became slotCanBeLoadedOrPushed (meaning freely generatable or spilled), so a spilled slot scores as regenerable-out-of-position (4) or surplus (3) instead of being popped unconditionally first.

@clonker
clonker marked this pull request as ready for review August 19, 2026 16:42
@clonker
clonker requested a review from blishko August 20, 2026 08:58
@clonker
clonker force-pushed the fix-ssa-cfg-shuffler-cycle-pop-spilled-top branch from 74376cf to 7933014 Compare August 20, 2026 08:58

@blishko blishko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, does not seem ideal. It makes some cases worse. On the other hand, this seems to be a niche situation, so I am not against merging.

@clonker
clonker force-pushed the fix-ssa-cfg-shuffler-cycle-pop-spilled-top branch from 7933014 to dd9238d Compare August 20, 2026 12:52
@clonker

clonker commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

At least it doesnt regress any of the semantic tests :)

@blishko

blishko commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

At least it doesnt regress any of the semantic tests :)

Probably because none of the semantic tests needs to spill? :)

@clonker

clonker commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

Probably because none of the semantic tests needs to spill? :)

Some of them do. But it's admittedly not a super representative stat.

@clonker
clonker merged commit f985208 into develop Aug 20, 2026
83 checks passed
@clonker
clonker deleted the fix-ssa-cfg-shuffler-cycle-pop-spilled-top branch August 20, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants