Conversation
This was broken when fgCanFastTailCall was changed to call fgInitArgInfo. fgInitArgInfo has side effects and will in some cases add arguments to the arg list. Specifically for calls to VSD, the VSD arg is added, however this case is treated specially for slow tailcalls and it does not expect the arg to be here. This targeted fix just removes this arg from the arg list.
|
@jkotas @nguerrera Is it possible for one of you to check if this PR fixes #27275? |
|
I think this likely fixes #25960, #25961, #26181 #26671, #26673 and #26829 also, although I have not checked. There may also be others (one possible symptom is a crash with |
|
/azp run coreclr-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@jakobbotsch please wait for the outerloop testing to finish before pushing a change. Also thank you for the fix! |
|
/azp list |
|
CI/CD Pipelines for this repository: |
|
It seems jitstress is failing in the new test with stack overflow in some cases. Probably there is some interaction in jitstress where we end up not being able to do a tailcall. I will investigate it tomorrow. Not sure what happened in the outerloop run that failed to build. The logs do not load. |
|
The problem seems to be STRESS_UNSAFE_BUFFER_CHECKS which makes one of the methods require a stack cookie check and we do not support tailcalls from such methods. I will disable the test in this mode for now, although I might submit a separate PR that turns this stress mode off for methods with explicit tailcalls in the future. |
|
The jitstress failures now appear to be previous issues so this should be good to go. @dotnet/jit-contrib should I mark this PR as fixing the issues from #27363 (comment)? I have not checked, but I don't think spending time on checking each individual bug is useful. I guess @VincentBu would not mind reopening those issues if they fail again. |
Confirmed. I pulled down this branch, built it, and replaced the shared framework bits in my repro sdk with the build. Problem fixed, thanks! |
Thank you for confirming, I have marked this as fixing that issue. |
|
@dotnet/jit-contrib ptal |
|
/azp run coreclr-ci |
|
/azp run coreclr-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@dotnet/jit-contrib I plan to merge this afternoon after the testing re-runs. Feel free take a look before then |
|
Testing passed. |
5 participants
This was broken in #20643 where fgCanFastTailCall was changed to call
fgInitArgInfo. fgInitArgInfo has side effects and will in some cases add
arguments to the arg list. Specifically for calls to VSD, the VSD arg is
added, however this case is treated specially for slow tailcalls and it
does not expect the arg to be here.
This targeted fix just removes this arg from the arg list.
In the future a better solution would be to refactor things so that
fgInitArgInfodoes not have side effects (rather than constructing the arg info).cc @jashook @BruceForstall @jkotas @nguerrera
Fixes #26311
Fixes #27275