Skip to content

Make IL Scanner report both variants#128107

Merged
eduardo-vp merged 6 commits into
dotnet:mainfrom
eduardo-vp:ilcscanner-report-both-variants
May 15, 2026
Merged

Make IL Scanner report both variants#128107
eduardo-vp merged 6 commits into
dotnet:mainfrom
eduardo-vp:ilcscanner-report-both-variants

Conversation

@eduardo-vp
Copy link
Copy Markdown
Member

IL Scanner currently reports only the async variant inside a runtime async method. While the JIT initially chooses this variant too, it can switch to the task returning variant if the async variant is a thunk. Make IL Scanner report both variants such that the JIT can choose either one.

Fixes #127179.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the NativeAOT IL scanner’s call dependency tracking for runtime-async “task await” patterns so it no longer reports only the async-variant method, but instead reports both the original method and its async variant, allowing the JIT to select either implementation during compilation.

Changes:

  • Stop rewriting the scanned call target to the async variant; instead, keep the original call target and track the async variant in parallel.
  • Add dependency reporting for the async variant alongside the original call target, including generic-lookup-related nodes where applicable.

Comment thread src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs Outdated
Comment thread src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
Comment thread src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs Outdated
Comment thread src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs Outdated
Copilot AI review requested due to automatic review settings May 13, 2026 21:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also re-enable the disabled test to validate this indeed fixes the problem.

@jakobbotsch are you okay with us still hardcoding certain things, like the await pattern match? I assume that one is not RyuJIT implementation detail, that one is more of an IL contract.

Comment thread src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs Outdated
Comment thread src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
Copilot AI review requested due to automatic review settings May 14, 2026 03:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@eduardo-vp
Copy link
Copy Markdown
Member Author

/ba-g unrelated failures

@eduardo-vp eduardo-vp merged commit 5d6e5b8 into dotnet:main May 15, 2026
113 of 118 checks passed
@jakobbotsch
Copy link
Copy Markdown
Member

@jakobbotsch are you okay with us still hardcoding certain things, like the await pattern match? I assume that one is not RyuJIT implementation detail, that one is more of an IL contract.

Sorry, I was OOF yesterday (public holiday here).

@EgorBo is currently working on an optimization that will see us switching to more async variants when possible. So NativeAOT should not assume that the only time we use async variants is based on the IL pattern.

For example, after inlining we really want the following to not use the task-returning thunk for C and refer directly to C:

async Task A()
{
  await B();
}

Task<int> B()
{
  return C();
}

[MethodImpl(MethodImplOptions.NoInlining)]
async Task<int> C()
{
  return 42;
}

I assume the only way to make that happen is that the ILScanner reports that the async variant of C is used when it sees it in B (for this issue my understanding is that C would need to be generic to see problems).

@MichalStrehovsky
Copy link
Copy Markdown
Member

I guess once that gets into way, we can relax more as needed. These issues will disappear if we ever get RyuJIT-as-a-scanner.

@jakobbotsch
Copy link
Copy Markdown
Member

I guess once that gets into way, we can relax more as needed. These issues will disappear if we ever get RyuJIT-as-a-scanner.

I imagine this would just be moving the conservativity into the JIT, right? It doesn't seem like there would be away to consistently predict during the scanning phase what happens in cases like the above, since we can later use facts obtained during scanning (like devirtualization that may result in new inlines).

@MichalStrehovsky
Copy link
Copy Markdown
Member

I think RyuJIT-as-a-scanner could in theory already assume some optimizations during scanning phase. But yes, I don't expect it to predict everything, especially not parts that can only be optimized once we have whole program view, so this will still have to be conservative, but maybe less so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ILC crash with runtime-async enabled calling JsonSerializer.DeserializeAsync<T>(...) from a generic method in NativeAOT

4 participants