Skip to content

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Apr 15, 2025

Instead of introducing a new mechanism to mark the async continuation register as busy until a kill we can use the delay-free mechanism, provided we take a bit care about where we insert the ref positions and about ensuring the async continuation ends up in the right spot.

Instead of introducing a new mechanism to mark the async continuation
register as busy until a kill we can keep use the delay-free mechanism,
provided we take a bit care about where we insert the ref positions and
about ensuring the async continuation ends up in the right spot.
@jakobbotsch jakobbotsch added the area-async Runtime generate async state machines label Apr 15, 2025
@jakobbotsch jakobbotsch changed the title Model async continuation definition via delay-free [RuntimeAsync] Model async continuation definition via delay-free Apr 15, 2025
@jakobbotsch
Copy link
Member Author

cc @VSadov

}

BlockRange().Remove(asyncCont);
BlockRange().InsertAfter(node, asyncCont);
Copy link
Member

@VSadov VSadov Apr 15, 2025

Choose a reason for hiding this comment

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

If we go backwards from continuation to find the call, why do we need to force the order? Would it be "after" anyways?"

Copy link
Member

Choose a reason for hiding this comment

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

I see - to make sure there is nothing in between.

{
MarkAsyncContinuationBusyForCall(call);
}

Copy link
Member

Choose a reason for hiding this comment

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

Should another MarkAsyncContinuationBusyForCall below be deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good catch

Copy link
Member

@VSadov VSadov left a comment

Choose a reason for hiding this comment

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

LGTM!

@jakobbotsch jakobbotsch merged commit 030e62c into dotnet:feature/async2-experiment Apr 16, 2025
1 of 7 checks passed
@jakobbotsch jakobbotsch deleted the delay-free-continuation branch April 16, 2025 19:48
@VSadov
Copy link
Member

VSadov commented Apr 16, 2025

After fetching from the async-experiment am getting an assert on x86.
Could it be related to the change?

 "E:\A2\runtimelab\artifacts\tests\coreclr\windows.x86.Checked\Tests\Core_Root\corerun.exe" -p "System.Reflection.Metadata.MetadataUpdater.IsSupported=false" -p "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization=true"  asynctests.dll
14:34:01.026 Running test: async\awaitingnotasync\awaitingnotasync.dll

Assert failure(PID 9104 [0x00002390], Thread: 47108 [0xb804]): Assertion failed '!"Missing BBF_PROF_WEIGHT flag"' in 'AwaitNotAsync:get_sProp():int' during 'Transform async' (IL size 37; hash 0x49da8969; FullOpts)

    File: E:\A2\runtimelab\src\coreclr\jit\fgprofile.cpp:4756
    Image: E:\A2\runtimelab\artifacts\tests\coreclr\windows.x86.Checked\Tests\Core_Root\corerun.exe

@jakobbotsch
Copy link
Member Author

jakobbotsch commented Apr 16, 2025

No, changes from this PR wouldn't have any effect during the async transformation. This is likely a latent bug that is now showing up after merging because of recent JIT flowgraph changes (e.g. dotnet/runtime#114016). I will take a look when time permits.

@VSadov
Copy link
Member

VSadov commented Apr 16, 2025

I think I have run x86 since the merge, but maybe I didn't or scenario needs some more conditions that were not there.

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

Labels

area-async Runtime generate async state machines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants