JIT: Disallow forward substitution of async calls #115936
Conversation
This can be forwarded into an overlapping byref temp, which is invalid IR. We could look for that condition specifically, but that's more complex. Do the simple thing for now and skip forward substituting trees containing async calls with a new method `gtTreeHasAsyncCall`. In the future when we have async contexts in SPMI collections we can try the more precise check and see if it makes a difference. Also introduce `impSpillAsyncCalls` using `gtTreeHasAsyncCall`, and resolve a TODO-Async using it. The opposite might also be possible: forward substituting a TYP_BYREF into a tree with an async call. However, I am not 100% sure that can create overlapping lifetimes, so let's wait and see if Fuzzlyn comes up with an example.
github-actions
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
cc @dotnet/jit-contrib PTAL @AndyAyersMS This passed CI previously but with some minor TP costs that I pushed a fix to mitigate. |
|
I realized that we also need to check for |
|
That |
Add a new helper `gtFindNodeInTree` to avoid multiple visitors.
|
Even that isn't enough, the call that the RET_EXPR points to can itself have RET_EXPR subtrees that recursively need to be processed. I'm going to revert the importer changes and just fix the forward sub one, and look at addressing the spilling inefficiency separately. |
|
/ba-g Android tests not picked up by Helix |
This can be forwarded into an overlapping byref temp, which is invalid IR.
We could look for that condition specifically, but that's more complex. Do the simple thing for now and skip forward substituting trees containing async calls with a new method
gtTreeHasAsyncCall. In the future when we have async contexts in SPMI collections we can try the more precise check and see if it makes a difference.The opposite might also be possible: forward substituting a TYP_BYREF into a tree with an async call. However, I am not 100% sure that can create overlapping lifetimes, so let's wait and see if Fuzzlyn comes up with an example.
Fix #115894