-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Intrinsically recognize Unsafe APIs in the scanner #82589
Conversation
Size optimization. If we're in shared code, call to an Unsafe intrinsic would bring generic lookup for the generic dictionary of the generic method. We would not be able to get rid of it later because it would be referenced from the other generic dictionary (the purpose of the scanning phase is to figure out how generic dictionaries will look like when we start code generation - this would be an unused slot). To add insult to injury, the dictionary would be empty most of the time because these generic methods don't actually do anything with their T. RyuJIT recognizes these intrinsically and it's not going to ask for the generic dictionary. Loose generic dictionaries actually add quite a bit of costs because we need to keep track of them in hash tables in the generated executable (type loader might need it, reflection might need them, etc.) Saves almost 1.1% on BasicMinimalApi (!!!).
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsSize optimization. If we're in shared code, call to an To add insult to injury, the dictionary would be empty most of the time because these generic methods don't actually do anything with their T. RyuJIT recognizes these intrinsically and it's not going to ask for the generic dictionary. Loose generic dictionaries actually add quite a bit of costs because we need to keep track of them in hash tables in the generated executable (type loader might need it, reflection might need them, etc.) Saves almost 1.1% on BasicMinimalApi (!!!). Cc @dotnet/ilc-contrib
|
@EgorBo are there any conditions where RyuJIT would not end up expand these in the importer? Do I need to mark them |
Currently, they're marked as "better to expand" meaning - in case of debug or minopts they won't be expanded. I think they can be marked as mustExpand anyway |
runtime/src/coreclr/jit/importercalls.cpp Line 2634 in afacfaf
|
There’s an issue (or might’ve been comment on one of the related PRs) that Jan put up asking us to make these recursive which will in turn make them Many hardware intrinsic APIs are notably in the same boat of not using their T, so it’s possible this applies to them as well Notably mustExpand really just exists to handle the recursion and needing a managed fallback impl. It doesn’t guarantee that all main usages are expanded in import as it may not handle all failure cases and there is the general case where we cannot expand intrinsics for a particular IT pattern today |
Going to go with #82589. |
Size optimization. If we're in shared code, call to an
Unsafe
intrinsic would bring generic lookup for the generic dictionary of the generic method. We would not be able to get rid of it later because it would be referenced from the other generic dictionary (the purpose of the scanning phase is to figure out how generic dictionaries will look like when we start code generation - this would be an unused slot).To add insult to injury, the dictionary would be empty most of the time because these generic methods don't actually do anything with their T.
RyuJIT recognizes these intrinsically and it's not going to ask for the generic dictionary.
Loose generic dictionaries actually add quite a bit of costs because we need to keep track of them in hash tables in the generated executable (type loader might need it, reflection might need them, etc.)
Saves almost 1.1% on BasicMinimalApi (!!!).
Cc @dotnet/ilc-contrib