JIT: recover types from helper calls and more #20447
Conversation
The jit needs to recover class handles in order to devirtualize and do other type-based optimizations. This change allows the jit to find the type for more trees: in particular, helper calls, intrinsics, and expanded static field accesses.
We don't want to optimize special methods that are used to inform crossgen about desirable generic instantiations. `CommonlyUsedGenericInstantiations` was already annotated but `CommonlyUsedWinRTRedirectedInterfaceStubs` wasn't. Also because `RuntimeType` is sealed calls through types are now often devirtualized. Name lookups on types are frequent, especially on error paths. The method `GetCachedName` looks like an attractive inline but simply expands into a larger sequence of two other calls. So block it from being inlined.
|
@briansull PTAL Per PMI: increases the rate of successful (virtual) call devirtualization from 0.182 to 0.195, and overall devirtualization from 0.156 to 0.166. About 3000 more virtual call sites devirtualized (out of ~190000). Jit-diffs (PMI, x64) shows: Size regressions are mostly from inlining methods with GUID arguments, where we subsequently fail to remove the struct copies. Size improvements and devirtualizations largely come from the fact that many type operations return |
|
OSX the usual file io pal test failures Ubuntu arm64 cross: all tests seemed to pass, but one test wrapper wouldn't load: |
|
I recently saw that "fail to load wrapper" in another run. It was a different wrapper. Can't remember the run (one of the cron jobs). |
src/jit/gentree.cpp
Outdated
| { | ||
| objClass = castHnd; | ||
| *isExact = false; | ||
| *isNonNull = isCastHelper; |
There was a problem hiding this comment.
CORINFO_HELP_CHKCAST* methods guarantee a non-null result only if the result type is a struct. Is this checked somehow before that already?
There was a problem hiding this comment.
It looks like in some cases the helpers are called only after a null check and direct MT compare have been done inline, but in other cases the helper is called directly, e.g. for array casts.
There was a problem hiding this comment.
No, it's not always checked. Thanks for catching this...
|
A few more xunit load failures in the Ubuntu arm cross job |
| // This method looks like an attractive inline but expands to two calls, | ||
| // neither of which can be inlined or optimized further. So block it | ||
| // from inlining. | ||
| [MethodImpl(MethodImplOptions.NoInlining)] |
There was a problem hiding this comment.
It would be nice if in the future the JIT could discover this fact and have a way to persist it so the next time we try to ngen/jit it we consult the database of known methods for this kind of information.
Perhaphs could add an option to ngen/crossgen to create a simplist .IBC data file that contained this information and then check it in along with the real .IBC data.
There was a problem hiding this comment.
I like the idea but do not know how we'd actually do something like this.
I think it might be easier to improve the inlining heuristics in this area (a relatively small method body has multiple sequential calls that are not themselves inline candidates). A strong enough conviction here would automatically propagate noinline into the runtime metadata when prejitting.
There was a problem hiding this comment.
Opened #20526 to track this idea.
|
CI not kicking off PR jobs, probably fallout from the github outage? [Edit: the jobs showed up finally] |
|
CoreFX test list seems to be messed up: |
|
Seems like this last entry needs {
"name": "System.Diagnostics.Tests",
"enabled": true,
"exclusions": {
"namespaces": null,
"classes": [
{
"name" : "System.Diagnostics.Tests.DebugTests",
"reason" : "refactoring Debug"
}
]
}
} |
|
After fixing the json issue via #20528 @dotnet-bot test Ubuntu x64 Checked CoreFX Tests and since OSX likes to randomly drop connections: @dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test |
|
Looks like more CI issues overnight. @dotnet-bot test this please |
|
Ubuntu runs hitting a "Failed to run DSL Script" error... |
|
Am going to retrigger again, hoping for better luck this time. @dotnet-bot test this please |
|
Ubuntu arm failure seems like more CI flakiness: |
|
Will give this one more shot: @dotnet-bot test Ubuntu arm Cross Checked no_tiered_compilation_innerloop Build and Test |
The jit needs to recover class handles in order to devirtualize and do other type-based optimizations. This change allows the jit to find the type for more trees: in particular, helper calls, intrinsics, and expanded static field accesses. Also, annotate a few methods to control jit optimization We don't want to optimize special methods that are used to inform crossgen about desirable generic instantiations. `CommonlyUsedGenericInstantiations` was already annotated but `CommonlyUsedWinRTRedirectedInterfaceStubs` wasn't. And because `RuntimeType` is sealed calls through types are now often devirtualized. Name lookups on types are frequent, especially on error paths. The method `GetCachedName` looks like an attractive inline but simply expands into a larger sequence of two other calls. So block it from being inlined.
The jit needs to recover class handles in order to devirtualize and do other type-based optimizations. This change allows the jit to find the type for more trees: in particular, helper calls, intrinsics, and expanded static field accesses. Also, annotate a few methods to control jit optimization We don't want to optimize special methods that are used to inform crossgen about desirable generic instantiations. `CommonlyUsedGenericInstantiations` was already annotated but `CommonlyUsedWinRTRedirectedInterfaceStubs` wasn't. And because `RuntimeType` is sealed calls through types are now often devirtualized. Name lookups on types are frequent, especially on error paths. The method `GetCachedName` looks like an attractive inline but simply expands into a larger sequence of two other calls. So block it from being inlined. Commit migrated from dotnet/coreclr@5af4d64
The jit needs to recover class handles in order to devirtualize and do other
type-based optimizations. This change allows the jit to find the type for more
trees: in particular, helper calls, intrinsics, and expanded static field accesses.
Also, annotate a few methods to control jit optimization
We don't want to optimize special methods that are used to inform crossgen
about desirable generic instantiations.
CommonlyUsedGenericInstantiationswas already annotated but
CommonlyUsedWinRTRedirectedInterfaceStubswasn't.Because
RuntimeTypeis sealed calls through types are now oftendevirtualized. Name lookups on types are frequent, especially on error paths.
The method
GetCachedNamelooks like an attractive inline but simply expandsinto a larger sequence of two other calls. So block it from being inlined.