Skip to content
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

Eliminate dead branches around typeof comparisons + Delete DebugSymbols #102391

Closed
wants to merge 3 commits into from

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented May 17, 2024

No description provided.

RyuJIT will already do dead branch elimination for `typeof(X) == typeof(Y)` patterns, but we couldn't do elimination around `foo == typeof(X)`. This fixes that using whole program knowledge - if we never saw a constructed `MT` for `X`, the comparison is not going to be true. Because it needs whole program, we still scan this dead branch so in the end this doesn't save much. We can eventually do better.

I'm doing this in `SubstitutedILProvider` instead of in RyuJIT: this is because we currently only reap a small benefit from this optimization due to it only happening during compilation phase. We need to do this during scanning as well. I think I can extend it to scannig. But the extension will require the optimization to 100% guaranteed happen during codegen. We cannot rely on whether RyuJIT will feel like it. `SubstitutedILProvider` is our way to ensure the optimization will happen no matter what - the IL from the branch will be gone and RyuJIT can at most remove the comparison (we don't mind much if it's left).
@jkotas jkotas added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it area-Infrastructure-libraries labels May 17, 2024
@jkotas jkotas changed the title Delete DebugSymbols Eliminate dead branches around typeof comparisons + Delete DebugSymbols May 17, 2024
Copy link
Contributor

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants