Avoid cctor check if we provably executed it - #131526
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
Pull request overview
This PR updates the AOT RyuJit JIT-EE interface to use initClass analysis (rather than a coarse “has lazy cctor” check) when reporting static field access requirements, enabling the JIT to elide redundant class-constructor checks when initialization is already provably satisfied (e.g., instance-method cases).
Changes:
- Refactors
initClassin the shared JitInterface to add aFieldDesc/MethodDescoverload and a handle-based wrapper. - Updates
ILCompiler.RyuJitgetFieldInfoto consultinitClassand (a) setCORINFO_FLG_FIELD_INITCLASSonly when needed and (b) choose relocatable static base access when init is not required.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | Adds an overload to run initClass logic directly on FieldDesc/MethodDesc, with a wrapper for handle-based entrypoints. |
| src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs | Uses initClass results in getFieldInfo to avoid unnecessary cctor triggering / helper usage for static field accesses. |
|
Worth noting that this can change behaviour for calling instance methods with null this, but ECMA explicitly permits skipping cctor then (still there could be code relying on it being executed). |
CoreCLR already skips it and calling instance methods with a null this seems like a scenario that we don't want to support at the first place. So I don't see any problem with this optimization. |
54c4e2e to
3575aa0
Compare
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
jkotas
left a comment
There was a problem hiding this comment.
Nice size savings MichalStrehovsky/rt-sz#254 !
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Sigh, too bad this missed .NET 11. As usual, .NET 12 will already be the best .NET by the time .NET 11 ships. This is now blocked on #132676 since we're hitting JIT assert in outerloop |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
For this program and the S method:
Before:
After: