Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Oct 14, 2025

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

KristofferC and others added 6 commits October 14, 2025 13:55
Fixes four related issues with SSA use counting in the optimizer:

1. populate_def_use_map! was incorrectly using `inst` instead of
   `inst[:stmt]` when iterating userefs, causing it to miss actual uses.

2. Early bailout in ScanStmt when encountering EnterNode or when there
   are no refinable effects caused incomplete SSA use counting in the
   TwoPhaseDefUseMap, leading to bounds errors when populate_def_use_map!
   tried to write beyond allocated array size.

3. scan_inconsistency! assumed the boundscheck argument was always a
   constant, but it can also be an SSA value that needs to be counted.
   The fix loops through all args to count SSAs in tpdum, but only
   checks non-boundscheck args (all except the last) when determining if
   the statement is inconsistent, preserving the special case that
   allows boundscheck to be inconsistent without tainting the overall
   statement.

4. check_inconsistentcy! was not convergent and not used correctly.

These fixes correspond to the patches in:
https://github.com/chalk-lab/Mooncake.jl/blob/main/src/interpreter/patch_for_319.jl

I imagine this maybe needs a test but not sure how to properly unit test
this without creating a full end-to-end test using `Base.code_ircode`
and so on.

The changes don't seem to have a performance impact reflected in the
benchmark job, so the fast paths are simply removed now. If they seem
impactful later, we can re-evaluate how to add those back correctly.

Fix #56193
Co-authored-by: Jules Merck <[email protected]>
Co-authored-by: Jameson Nash <[email protected]>
With rebasing help by Claude Code
… not needed with bracket paste being supported (#59827)
@pull pull bot locked and limited conversation to collaborators Oct 14, 2025
@pull pull bot added the ⤵️ pull label Oct 14, 2025
@pull pull bot merged commit c171ddb into MLH-Fellowship:master Oct 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants