Constrained mask should take into account busy registers #85933
Conversation
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsWhen we constrained the registers under JitStressRegs, check if the constrained masks is not left with "busy registers" because in that case, we will not be able to allocate register. Relax that and do not constraint if they coincides with "busy registers". Fixes: #85627
|
|
I am seeing superpmi-replay failures similar to one reported in #59916. |
|
@dotnet/jit-contrib |
BruceForstall
left a comment
BruceForstall
left a comment
There was a problem hiding this comment.
Mostly nits but one question
src/coreclr/jit/codegenxarch.cpp
Outdated
| lclNum2 = op2Skip->AsLclVarCommon()->GetLclNum(); | ||
| } | ||
| #endif | ||
| noway_assert(GenTree::OperIsCommutative(oper) || (lclNum1 == lclNum2)); |
There was a problem hiding this comment.
I realized this can't be a noway_assert now: it would mean we're asserting different things in Release and Debug: and the case where you're asserting lclNum1 == lclNum2 would fail in Release.
ghost
added
the
needs-author-action
ghost
removed
the
needs-author-action
2 participants
When we constrained the registers under JitStressRegs, check if the constrained masks is not left with "busy registers" because in that case, we will not be able to allocate register. Relax that and do not constraint if they coincides with "busy registers".
Fixes: #85627
(Most likely) fixes: #76382
Also noticed a problem where we were not taking into account
SUB_HIandLSH_HIfor RMW analysis. Doing that discovered that we should update the assert about Commutative() to take into account if both operands are same. Both of these will fix the following issue:Fixes: #59916