Skip to content

slot2ssa: Fix spurious φᶜ edges#51199

Merged
topolarity merged 3 commits intoJuliaLang:masterfrom
topolarity:fix-51159
Sep 6, 2023
Merged

slot2ssa: Fix spurious φᶜ edges#51199
topolarity merged 3 commits intoJuliaLang:masterfrom
topolarity:fix-51159

Conversation

@topolarity
Copy link
Copy Markdown
Member

@topolarity topolarity commented Sep 5, 2023

The unreachable here seems to be caused by the fact that (as of #50943) we re-use a more narrow type from Inference that correctly ignores these edges, but when inserting the φᶜ node in slot2reg we were including extra edges that never get exercised at runtime.

I'm not sure why this causes us to hit an unreachable, since the narrow type from inference is technically still valid (the catch block will never observe these spurious assignments at runtime), but this seems to fix the issue and anyway improves the quality of the IRCode generated by slot2ssa.

Resolves #51159

The unreachable here seems to be caused by the fact that (as of JuliaLang#50299) we
re-use a more narrow type from Inference that correctly ignores these
edges, but when inserting the `φᶜ` node in `slot2reg` we were including
extra edges that never get exercised at runtime.

I'm not sure _why_ this causes us to hit an unreachable, since the narrow
type from inference is technically still valid (the catch block will never
observe these spurious assignments at runtime), but this seems to fix the
issue and anyway improves the quality of the IRCode generated by `slot2ssa`.

Resolves JuliaLang#51159
@topolarity topolarity requested a review from Keno September 5, 2023 15:44
This variable is currently always zero.
@vtjnash
Copy link
Copy Markdown
Member

vtjnash commented Sep 5, 2023

φᶜ assignment happens semantically at the earlier ɣ, just as φ assignment happens at the preceding edge. The actual φᶜ node is just a placeholder for dominator analysis and ssa numbering.

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Sep 5, 2023
@topolarity
Copy link
Copy Markdown
Member Author

φᶜ assignment happens semantically at the earlier ɣ

Do you mean that was happening even though the ɣ was outside of any try-catch block?

@topolarity topolarity merged commit aeae142 into JuliaLang:master Sep 6, 2023
@topolarity topolarity deleted the fix-51159 branch September 6, 2023 19:02
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Sep 8, 2023
# Issue #51159 - Unreachable reached in try-catch block
function f_with_early_try_catch_exit()
result = false
for i in 3
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this meant to be for i in 1:3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unreachable reached due to TypedSlot removal

5 participants