-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Enable compilation of async thunks in composite mode #126904
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
Open
jtschuster
wants to merge
79
commits into
dotnet:main
Choose a base branch
from
jtschuster:composite-async-variants
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
79 commits
Select commit
Hold shift + click to select a range
9a1ddd8
Add crossgen CI analysis agentic workflow (#9)
jtschuster 87296e1
Crossgen2 ci triage workflow (#10)
jtschuster 130a3c0
Crossgen2 ci triage workflow (#19)
jtschuster 1606998
Add cross-module R2R reference resolution tests
jtschuster 1493728
Add runtime-async method variants to cross-module resolution tests
jtschuster 70ad8f0
Add R2R validation tool for cross-module resolution tests
jtschuster 1575e66
Extract crossgen2 precommands into shared .targets file
jtschuster ee26040
Refactor crossgen2 orchestration into C# console app
jtschuster 67ffdd6
Output runcrossgen to CORE_ROOT/runcrossgen/
jtschuster d5b2d3a
Use host SDK RID for runcrossgen build-time tool
jtschuster 5514f1b
Merge branch 'dotnet:main' into main
jtschuster 8029183
WIP: Add ILCompiler.ReadyToRun.Tests project
jtschuster 3e1a6c8
Fix R2R test validation and remove opt-async-methods
jtschuster 183262c
Add runtime-async functional tests for R2R
jtschuster 7bb35e5
Refactor R2R test model: rename DependencyInfo to CompiledAssembly, a…
jtschuster 804ec55
Replace R2RExpectations with Action<ReadyToRunReader> validation call…
jtschuster 25ee0be
Fix R2R test runner: --opt-cross-module args and composite output naming
jtschuster b2e0f5b
Add InliningInfo2 structured parsing and method-targeted assertions
jtschuster c8c6a35
Add 10 new R2R cross-module/async/composite test cases
jtschuster 805f042
Add test source files for new R2R test cases
jtschuster d90a6cd
Fix composite tests: add --optimize flag and cross-assembly inline as…
jtschuster 67e8d19
Fix code review issues: parser bug, deadlock, file handle leak
jtschuster 2ab55e0
Add CrossModuleGenericMultiInliner test for cross-module inliner parsing
jtschuster 03d71ec
Validate resolved inliner names in HasCrossModuleInliners
jtschuster 22e55bd
Remove unused Utility.GetName() from CrossModuleGenericLib test
jtschuster b6d5227
Remove session state and planning files from tracking
jtschuster 61ea4a9
Remove crossmoduleresolution tests from src/tests/readytorun
jtschuster 537cd1b
Fix stale and inaccurate comments across R2R test files
jtschuster 54a3bb6
Enable async method variants in composite mode
jtschuster 0aa5103
Remove unused using System directive in MultiStepAsyncConsumer
jtschuster 40a61b2
Revert async method variant enablement in composite mode
jtschuster ce4ca00
WIP: Execution infrastructure, ActiveIssue, and MethodDefEntryPointsT…
jtschuster cc18125
Crossgen2 CI triage: cross-reference runtime pipeline and restrict to…
Copilot fa70608
Don't use MutableModuleWrappedMethodIL for async thunks
jtschuster e0613cc
Merge branch 'main' of https://github.com/jtschuster/runtime into com…
jtschuster 8dc7ab0
Force OwningTypeNotDerivedFromToken for method references in async th…
jtschuster 849c73b
Revert NativeArray changes
jtschuster b53da9f
Undo all changes related to the test infra change this was branched off
jtschuster 1bab3b6
Remove .github changes
jtschuster 505abeb
Revert more changes
jtschuster 8edd9b9
Revert changes to .github to the right version?
jtschuster b733fad
one more try at the .github dir
jtschuster 3d0f41d
Merge branch 'main' of https://github.com/dotnet/runtime into composi…
jtschuster 8b63902
Extract EnsureDefTokensAreAvailable and related method to ExternalRef…
jtschuster f2704fb
Add details to stale IL-CG2 warning
jtschuster 9fc88fa
Address copilot feedback
jtschuster 182b580
Merge branch 'main' into composite-async-variants
jtschuster 1aa38f9
Fix PR comments
jtschuster c66c646
Properly get the EcmaMethod in CreateCrossModuleInlineableTokensForIL…
jtschuster 303dcdc
Get IL for the (possible instantiated) method, not the definition
jtschuster bb834fe
Merge branch 'main' of https://github.com/dotnet/runtime into composi…
jtschuster 0ef7dfc
Expand R2R composite-async test coverage and refactor R2RAssert to bo…
jtschuster 0f5280d
Add CompositeAsyncGenericTypes regression test
jtschuster c224b67
Dedupe R2R test fixtures: LocalsCapturedAcrossAwait & InlinableAsyncM…
jtschuster ef70780
Dedupe R2R async devirt test fixtures: AwaitsThroughInterface
jtschuster 49bb774
Dedupe R2R transitive test fixtures: share leaf between sync + async …
jtschuster d3fe5b8
Rename R2R basic-inlining test fixtures to describe code, not scenario
jtschuster 4998999
Rename remaining R2R test fixtures to describe code, not scenario
jtschuster 2203bd7
Rename R2R test deps to <WhatsInFile>.cs; merge InlinableAsyncMethods…
jtschuster fdcb6c2
Revert R2R test fixture dedup/rename to keep follow-up PR isolated
jtschuster f641339
Replace ad-hoc header comments in R2R test fixtures with MIT license …
jtschuster 22d2b51
Restore original headers on R2R test fixtures unchanged by this branch
jtschuster 0b97456
Remove redundant OwningType comparison in MethodWithToken.CompareTo
jtschuster 4251106
Clarify EnsureDefTokensAreAvailable XML docs
jtschuster 4f380b5
Add positive composite cross-module inlining test
jtschuster faa8ce1
Simplify test comments, remove redundant test
jtschuster b85e2d2
Refine documentation for CompositeProducesCrossModuleInliningInfoForE…
jtschuster 37d3d6a
Include OwningTypeNotDerivedFromToken in FieldWithToken.Equals
jtschuster 04352ad
Forward strippedInstantiation to FieldWithToken in shared generic lookup
jtschuster 47e4e25
Assert strippedInstantiation is false in FieldDesc helper-arg path
jtschuster 99b84bc
Fix FieldWithToken.CompareTo
jtschuster 7f15da0
Add asserts, revert extra change
jtschuster 81dac5f
Remove redundant CreatingTokensForAsyncMethod set in EnsureAsyncThunk…
jtschuster cefc723
Merge branch 'main' of https://github.com/dotnet/runtime into composi…
jtschuster a902c89
Get Field token with compilation ModuleTokenResolver
jtschuster 9a6f3f7
Use IL availability as determining factor for whether to create Mutab…
jtschuster 8290b2c
Enable getting PrimaryMethodDesc for ReturnDroppingThunks
jtschuster f38e8da
Compare forceOwningTypeNotDerivedFromToken in XWithTokien types
jtschuster ad8fa51
Allow empty string to be resolved for inlinees of ILStubs
jtschuster File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,6 +35,7 @@ internal class TypeSystemMetadataEmitter | |||||||||||||||||||||||||||||||||||||||||
| _metadataBuilder = new MetadataBuilder(); | ||||||||||||||||||||||||||||||||||||||||||
| _ilBuilder = new BlobBuilder(); | ||||||||||||||||||||||||||||||||||||||||||
| _methodBodyStream = new MethodBodyStreamEncoder(_ilBuilder); | ||||||||||||||||||||||||||||||||||||||||||
| _ = _metadataBuilder.GetOrAddString(string.Empty); | ||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't do anything: Lines 440 to 459 in 0b9b47d
|
||||||||||||||||||||||||||||||||||||||||||
| StringHandle assemblyNameHandle = _metadataBuilder.GetOrAddString(assemblyName.Name); | ||||||||||||||||||||||||||||||||||||||||||
| if (assemblyName.CultureName != null) | ||||||||||||||||||||||||||||||||||||||||||
| throw new ArgumentException("assemblyName"); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
44 changes: 44 additions & 0 deletions
44
...tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/CrossModuleInlining/AsyncInlineCallers.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Runtime.CompilerServices; | ||
| using System.Threading.Tasks; | ||
|
|
||
| public static class AsyncInlineCallers | ||
| { | ||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| public static async Task CallReturnTaskNoAwait() | ||
| { | ||
| await AsyncInlineCandidatesLib.ReturnTaskNoAwait(); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| public static async Task<int> CallReturnTaskPrimitiveNoAwait() | ||
| { | ||
| return await AsyncInlineCandidatesLib.ReturnTaskPrimitiveNoAwait(); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| public static async Task<string> CallReturnTaskClassNoAwait() | ||
| { | ||
| return await AsyncInlineCandidatesLib.ReturnTaskClassNoAwait(); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| public static async Task CallReturnTaskWithAwait() | ||
| { | ||
| await AsyncInlineCandidatesLib.ReturnTaskWithAwait(); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| public static async Task<int> CallReturnTaskPrimitiveWithAwait() | ||
| { | ||
| return await AsyncInlineCandidatesLib.ReturnTaskPrimitiveWithAwait(); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| public static async Task<string> CallReturnTaskClassWithAwait() | ||
| { | ||
| return await AsyncInlineCandidatesLib.ReturnTaskClassWithAwait(); | ||
| } | ||
| } |
43 changes: 43 additions & 0 deletions
43
...r.ReadyToRun.Tests/TestCases/CrossModuleInlining/Dependencies/AsyncInlineCandidatesLib.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Runtime.CompilerServices; | ||
| using System.Threading.Tasks; | ||
|
|
||
| public static class AsyncInlineCandidatesLib | ||
| { | ||
| // --- Awaitless variants: should be inlinable --- | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static async Task ReturnTaskNoAwait() | ||
| { | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static async Task<int> ReturnTaskPrimitiveNoAwait() => 42; | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static async Task<string> ReturnTaskClassNoAwait() => "no_await"; | ||
|
|
||
| // --- Variants containing an actual await: cannot be inlined by the JIT --- | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static async Task ReturnTaskWithAwait() | ||
| { | ||
| await Task.Yield(); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static async Task<int> ReturnTaskPrimitiveWithAwait() | ||
| { | ||
| await Task.Yield(); | ||
| return 42; | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static async Task<string> ReturnTaskClassWithAwait() | ||
| { | ||
| await Task.Yield(); | ||
| return "with_await"; | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HandleToModuleToken(..., out strippedInstantiation)can legitimately setstrippedInstantiation=truefor faux MethodIL (e.g., compiler-generated IL stubs) when it strips method instantiation (see CorInfoImpl.ReadyToRun.cs where it compares primary vs typical method definition). SincerecordTokenexplicitly allowsIMethodTokensAreUseableInCompilation,Debug.Assert(!strippedInstantiation)here can fire when compiling those stubs (including async thunk scenarios this PR enables). Consider removing this assert or adjusting the logic to tolerate stripped instantiation (and rely on the MethodDesc/type context when building signatures).