Properly handle byrefs in tailcall helper stubs #41815
Conversation
Switch to using ByReference instead of using stind.i/ldind.i and relying on the JIT to report the locations being moved between. Fixes dotnet#41555
Dotnet-GitSync-Bot
added
the
area-CodeGen-coreclr
|
@BruceForstall assuming tests pass, can you check if this fixes #41555? |
I was never able to reliably repro the problem, so I won't be able to "prove" the fix works that way. I looked at the generated code + GC info for the "Call" stub for the 41555 test case on Windows arm32, and it looks good to me: the byrefs I annotated in the bug as needing GC tracking are appropriately tracked. If you're happy with this change as is, I'm inclined to trigger outerloop + gcstress0x3-gcstress0xc + gcstress-extra legs. That's expensive and time consuming, but it's the best way, IMO, to see that the issue doesn't repro in the CI where it was (somewhat) reliably repro'ing (and check for regressions). |
Thanks for checking! Triggering those legs sounds good to me. |
|
Triggered: outerloop: https://dev.azure.com/dnceng/public/_build/results?buildId=800825&view=results |
|
outerloop failures are #41853, gcstress failures are a few timeouts (that's unfortunately common). The |
|
/backport to release/5.0-rc2 |
|
Started backporting to release/5.0-rc2: https://github.com/dotnet/runtime/actions/runs/239756050 |
ghost
locked as resolved and limited conversation to collaborators
Switch to using ByReference instead of using stind.i/ldind.i and relying
on the JIT to report the locations being moved between.
Fixes #41555
cc @BruceForstall @jkotas