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.

Sign up for GitHub

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

Jump to bottom

Make it possible to eliminate branches guarded by IsEnum #100399

Merged
merged 3 commits into from May 13, 2024
Merged

Make it possible to eliminate branches guarded by IsEnum #100399

merged 3 commits into from May 13, 2024

Conversation

MichalStrehovsky
Copy link
Member

We use this check in CoreLib and currently it includes some unreachable code with a big closure.

@MichalStrehovsky
Make it possible to eliminate branches guarded by IsEnum
417b631 
We use this check in CoreLib and currently it includes some unreachable code with a big closure.
@github-actions github-actions bot mentioned this pull request Mar 28, 2024
Closed
@MichalPetryka
Copy link
Contributor

Does handling the internal IsActualEnum here too make sense?

@MichalStrehovsky
Copy link
Member Author

Does handling the internal IsActualEnum here too make sense?

This only helps things like typeof(T).IsEnum. It doesn't do anything for when we have a System.Type from who-knows-where. If we use IsActualEnum on typeof(T) anywhere, we should just switch it to IsEnum.

@build-analysis build-analysis bot mentioned this pull request May 6, 2024
Closed
@Sergio0694
Copy link
Contributor

Wondering if this could also give us some size savings in CsWinRT. Eg. we have a few branches on this there (example):

image

@MichalStrehovsky
Copy link
Member Author

Wondering if this could also give us some size savings in CsWinRT. Eg. we have a few branches on this there (example):

Yes, that would probably help here.

I'll merge this then. I wasn't fully convinced we really want this without the rest. Despite the impression I'm giving here, I don't particularly like we do this IL rewriting.

Size statistics

Pull request #100399

Project Size before Size after Difference
avalonia.app-linux 24496560 24492464 -4096
avalonia.app-windows 22036480 22031360 -5120
hello-linux 1377040 1348240 -28800
hello-minimal-linux 1110696 1085992 -24704
hello-minimal-windows 885248 864256 -20992
hello-windows 1133056 1109504 -23552
kestrel-minimal-linux 5772000 5767904 -4096
kestrel-minimal-windows 5072384 5070848 -1536
reflection-linux 2294656 2294656 0
reflection-windows 1944576 1944064 -512
webapiaot-linux 10249144 10228632 -20512
webapiaot-windows 9206272 9188352 -17920

@MichalStrehovsky MichalStrehovsky merged commit 1be664e into dotnet:main May 13, 2024
90 of 93 checks passed
@MichalStrehovsky MichalStrehovsky deleted the isenum branch May 13, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkotas jkotas jkotas approved these changes

Assignees

@MichalStrehovsky MichalStrehovsky

Labels
area-NativeAOT-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@MichalStrehovsky @MichalPetryka @Sergio0694 @jkotas