-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consume regs for BlkOpKindLoop #97014
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
/azp run runtime-coreclr libraries-jitstress2-jitstressregs, runtime-coreclr jitstress2-jitstressregs |
@dotnet/jit-contrib @EgorBo PTAL |
Azure Pipelines successfully started running 2 pipeline(s). |
Does it lead to bad codegen? (because that PR was backported to 8.0) or just consumes unnecessary reg? CI failures seem related? |
It should consume the unnecessary reg, but perhaps that is leading to unused register and that is getting used somewhere below with wrong value. Let me investigate. However, I think for arm64, if we know that we will be using
Taking a look |
I see that they are marked contained https://github.com/dotnet/runtime/pull/95530/files#diff-547b75c8f8dd006693805c30d7a0e2ddcdd6b10691383dfbb16e3ed0c50f236aR621-R628 |
ah, i think the short-circuit for stress is missing the containment marking. |
Cool, then it doesn't need any backport to 8.0 🙂 LGTM if CI agrees |
/azp run runtime-coreclr libraries-jitstress2-jitstressregs, runtime-coreclr jitstress2-jitstressregs |
Azure Pipelines successfully started running 2 pipeline(s). |
Failures seems unrelated because the changes are on arm64, and failures are x86. |
* Consume regs for BlkOpKindLoop * Mark the node as contained * undo Program.cs commit
In #95530, we forgot to consume the register that leads to a temp register around.
Fixes: #96599