Recognize Task<->ValueTask adaptions in async versions

[jakobbotsch]

Support looking through return new ValueTask(TaskReturningFunction()) and return ValueTaskReturningFunction().AsTask() in async versions. These can be transformed into async calls.

This is a common pattern in the BCL, e.g.:

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs

Line 268 in 06362e8

return ReceiveAsync(buffer, socketFlags, fromNetworkStream, default).AsTask();

runtime/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs

Line 176 in 5ee89e4

return new ValueTask(WriteAsyncCore(buffer, cancellationToken));

Example:

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static ValueTask TestTaskToValueTask()
    {
        return new ValueTask(TestTask());
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static async Task TestTask()
    {
        await Task.Yield();
    }

Diff:

 G_M37697_IG01:
-       push     rbp
-       sub      rsp, 80
-       lea      rbp, [rsp+0x50]
-       vxorps   xmm4, xmm4, xmm4
-       vmovdqu  ymmword ptr [rbp-0x30], ymm4
-       vmovdqa  xmmword ptr [rbp-0x10], xmm4
-       mov      gword ptr [rbp+0x10], rcx
-						;; size=28 bbWeight=1 PerfScore 5.08
+       sub      rsp, 40
+						;; size=4 bbWeight=1 PerfScore 0.25
 G_M37697_IG02:
-       vxorps   xmm0, xmm0, xmm0
-       vmovdqu  xmmword ptr [rbp-0x10], xmm0
-       call     [AsyncMicro.Program:TestTask():System.Threading.Tasks.Task]
-       mov      gword ptr [rbp-0x18], rax
-       mov      rdx, gword ptr [rbp-0x18]
-       lea      rcx, [rbp-0x10]
-       call     [System.Threading.Tasks.ValueTask:.ctor(System.Threading.Tasks.Task):this]
-						;; size=33 bbWeight=1 PerfScore 9.83
-G_M37697_IG03:
-       vmovdqu  xmm0, xmmword ptr [rbp-0x10]
-       vmovdqu  xmmword ptr [rbp-0x28], xmm0
-						;; size=10 bbWeight=1 PerfScore 2.00
-G_M37697_IG04:
-       lea      rdx, [rbp-0x28]
        xor      rcx, rcx
-       call     [System.Runtime.CompilerServices.AsyncHelpers:TransparentAwaitWithResult(System.Threading.Tasks.ValueTask)]
-       mov      gword ptr [rbp-0x30], rcx
-       cmp      gword ptr [rbp-0x30], 0
-       jne      SHORT G_M37697_IG06
+       call     [AsyncMicro.Program:TestTask()]
+       test     rcx, rcx
+       jne      SHORT G_M37697_IG04
        xor      ecx, ecx
-						;; size=25 bbWeight=1 PerfScore 7.00
-G_M37697_IG05:
-       add      rsp, 80
-       pop      rbp
+						;; size=15 bbWeight=1 PerfScore 4.75
+G_M37697_IG03:
+       add      rsp, 40
        ret      
-						;; size=6 bbWeight=1 PerfScore 1.75
-G_M37697_IG06:
-       mov      rax, gword ptr [rbp-0x30]
-       mov      rcx, rax
-						;; size=7 bbWeight=0 PerfScore 0.00
-G_M37697_IG07:
-       add      rsp, 80
-       pop      rbp
+						;; size=5 bbWeight=1 PerfScore 1.25
+G_M37697_IG04:
+       add      rsp, 40
        ret      
-						;; size=6 bbWeight=0 PerfScore 0.00
+						;; size=5 bbWeight=0 PerfScore 0.00
 
-; Total bytes of code 115, prolog size 24, PerfScore 25.67, instruction count 31, allocated bytes for code 115 (MethodHash=6db76cbe) for method AsyncMicro.Program:TestTaskToValueTask() (Tier0)
+; Total bytes of code 29, prolog size 4, PerfScore 6.25, instruction count 10, allocated bytes for code 29 (MethodHash=6db76cbe) for method AsyncMicro.Program:TestTaskToValueTask() (FullOpts)
 ; ============================================================
 

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR extends CoreCLR “runtime-async / async-version” recognition so the JIT can see through common Task↔ValueTask adapters (new ValueTask(task) and valueTask.AsTask()) and treat them as direct async calls, reducing wrapper overhead. It also adds tests intended to cover these adapter patterns.

Changes:

• Mark ValueTask / ValueTask<T> Task-based constructors and AsTask() methods as [Intrinsic] so the JIT can pattern-match them.
• Teach the JIT importer to recognize async-version tail patterns involving ValueTask adapters and propagate a new “adapted” flag into async-call lowering and continuation flags.
• Add/adjust async tests/projects for adapter scenarios and align the IL project’s assembly naming.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/tests/async/async-versions/async-versions.ilproj	Adds an IL SDK project for the IL-authored async-version tests; removes injected C# helper attribute source.
src/tests/async/async-versions/async-versions.il	Renames the IL assembly to match the project name (async-versions).
src/tests/async/async-versions-task-adapters/async-versions-task-adapters.csproj	New test project to validate Task↔ValueTask adapter behavior.
src/tests/async/async-versions-task-adapters/async-versions-task-adapters.cs	New xUnit tests covering new ValueTask(Task), ValueTask.AsTask(), and ValueTask<Task<int>> overload binding behavior.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/ValueTask.cs	Adds [Intrinsic] to relevant constructors/AsTask() overloads to enable JIT recognition.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds a new continuation flag bit and shifts bitfield layout constants.
src/coreclr/jit/namedintrinsiclist.h	Adds new NamedIntrinsic IDs for ValueTask/ValueTask<T> ctor and AsTask.
src/coreclr/jit/importercalls.cpp	Blocks certain tail-await optimizations/inlining for adapted patterns; wires new intrinsic lookups.
src/coreclr/jit/importer.cpp	Factors stloc/ldloca matching; adds async-version tail-call pattern matching for AsTask() and newobj ValueTask(..).
src/coreclr/jit/gentree.h	Adds AsyncCallInfo.IsValueTaskAsTask marker.
src/coreclr/jit/compiler.h	Adds PREFIX_IS_ADAPTED_FROM_VALUETASK prefix flag.
src/coreclr/jit/async.cpp	Sets the new continuation flag based on IsValueTaskAsTask.
src/coreclr/inc/corinfo.h	Adds CORINFO_CONTINUATION_VALUETASK_ADAPTED_TO_TASK and updates bitfield layout constants.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

[AndyAyersMS]

JIT changes look ok.

How robust are all these patterns? Is missing one just a lost optimization?

The interpreter pattern match machinery looks appealing, though I suppose in the JIT we also need to bind on certain bits of info along the way and/ or do further valuation so maybe it can't be this simple?

[jakobbotsch]

How robust are all these patterns? Is missing one just a lost optimization?

Yes, it just results in losing the direct runtime async call, meaning we materialize a Task object and await it via AsyncHelpers.TransparentAwait. That has the same behavior as the direct runtime async call.

It would be interesting to do this optimization on JIT IR too by pushing AsyncHelpers.Await/TransparentAwait up through phis/defs (as side effects allow) and then folding that when possible. For example, I came across this example the other day:

runtime/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs

Lines 753 to 771 in 9d70f01

	ValueTask<ReadResult> result;
	lock (SyncObj)
	{
	_readerAwaitable.BeginOperation(token, s_signalReaderAwaitable, this);
	// If the awaitable is already complete then return the value result directly
	if (_readerAwaitable.IsCompleted)
	{
	GetReadResult(out ReadResult readResult);
	result = new ValueTask<ReadResult>(readResult);
	}
	else
	{
	// Otherwise it's async
	result = new ValueTask<ReadResult>(_reader, token: 0);
	}
	}
	return result;

Conceptually, this gets transformed to

            ValueTask<ReadResult> result;
            lock (SyncObj)
            {
                _readerAwaitable.BeginOperation(token, s_signalReaderAwaitable, this);

                // If the awaitable is already complete then return the value result directly
                if (_readerAwaitable.IsCompleted)
                {
                    GetReadResult(out ReadResult readResult);
                    result = new ValueTask<ReadResult>(readResult);
                }
                else
                {
                    // Otherwise it's async
                    result = new ValueTask<ReadResult>(_reader, token: 0);
                }
            }

            return AsyncHelpers.TransparentAwait(result);

in the async version of this method. It would have better performance if written as:

ReadResult syncResult = default;
IValueTaskSource<ReadResult> asyncResult = null;

lock (SyncObj)
{
     _readerAwaitable.BeginOperation(token, s_signalReaderAwaitable, this); 
  
     // If the awaitable is already complete then return the value result directly 
     if (_readerAwaitable.IsCompleted) 
     { 
         GetReadResult(out syncResult);
         goto ReturnSync;
     } 
     else 
     { 
         // Otherwise it's async 
        asyncResult = _reader;
        goto ReturnAsync;
     } 
}

ReturnSync:
return new ValueTask(syncResult);

ReturnAsync:
return new ValueTask(_reader, token: 0);

since we know how to fold AsyncHelpers.TransparentAwait(new ValueTask(syncResult)) => syncResult.

Ideally we would do something like this automatically. In this case there is actually intervening side effect (exiting the lock), but I would expect there to be cases too where we could switch task-returning calls to runtime async calls if we did this.

The interpreter pattern match machinery looks appealing, though I suppose in the JIT we also need to bind on certain bits of info along the way and/ or do further valuation so maybe it can't be this simple?

I like it for the simple things, but in this particular case we need to match stloc+ldloca both of which have multiple encodings, and that ends up being quite messy. I think a bit more flexibility would be needed to make this really convenient to use for complex patterns. I think as it is I would be more inclined to abstract the JIT's pattern matching code for these cases and then use it from interpreter, JIT and NativeAOT.

[jakobbotsch]

Looks like I have some NativeAOT test failures to look at. Fixed in 7ccc165

[BrzVlad]

Interpreter LGTM

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

Last nativeaot-outerloop failure is the one being fixed in #130618

[VSadov]

General question. Is there a reason to use ValueTask in the first place now that we have runtime async ? Doesn't runtime async make the usefullness of ValueTask obsolete by avoiding task allocation for the non-blocking path ?

ValueTask is better than Task in two scenarios (and they are exactly the scenarios when VT does not wrap Task):

• when VT wraps a completed result.
  Yes Task can do that too and common completed Tasks can even be cached, but VT can wrap any T result with no allocations.
• wrap a ValueTaskSource in a way that can be versioned and re-armed.
  A Task, once complete, is complete and cannot be rearmed. VT just wraps a tuple {source, version}, so you can get numerous versioned results from the same source, some complete, some not yet - all with no allocations.

The "completed result" scenario is naturally matched quite well in runtime async.

The "async result generator" scenario is done via the same {source, version} abstraction. We just had to make runtime async familiar with existing ValueTaskSource APIs. Not sure how much different this scenario would be if we had rt async from the beginning.

[VSadov]

LGTM!

[jakobbotsch]

/ba-g Unknown failure is the one fixed in #130618

[jakobbotsch]

ValueTask is better than Task in two scenarios (and they are exactly the scenarios when VT does not wrap Task):

* when VT wraps a completed result.
  Yes Task can do that too and common completed Tasks can even be cached, but VT can wrap any T result with no allocations.

* wrap a ValueTaskSource in a way that can be versioned and re-armed.
  A Task, once complete, is complete and cannot be rearmed. VT just wraps a tuple `{source, version}`, so you can get numerous versioned results from the same source, some complete, some not yet - all with no allocations.

The "completed result" scenario is naturally matched quite well in runtime async.

Just to be clear: I think @VSadov is talking about the scenarios that led to ValueTask's existence, not the state of things with runtime async. For strictly runtime async calls there is no difference between ValueTask and Task. They will result in the same codegen under-the-hood for both synchronously finishing and asynchronously finishing work. There would only be a difference if you explicitly materialize a Task or ValueTask from a runtime async method (i.e. if you don't await it immediately).