Fix an over-constrained use of a byte reg #41004
Conversation
Dotnet-GitSync-Bot
added
the
area-CodeGen-coreclr
| if (internalIsByte && (useCount >= 4)) | ||
| { | ||
| noway_assert(internalIntDef != nullptr); | ||
| internalIntDef->registerAssignment = RBM_RAX; |
There was a problem hiding this comment.
This is the actual fix.
src/coreclr/src/jit/lsraxarch.cpp
Outdated
| // we must have less than 4 sources. | ||
| // If we have 4 or more sources, and require a byteable internal register, we need to reserve | ||
| // one explicitly (see BuildBlockStore()). | ||
| assert(srcCount < 4); |
There was a problem hiding this comment.
This and the other assert below are added because these are the only other semi-complex cases where we have byteable restrictions. I've convinced myself that we will never have more than 3 uses in these cases, so a simple assert is useful and sufficient (we don't need to check that we actually have a byteable register source).
|
@dotnet/jit-contrib PTAL |
There was a problem hiding this comment.
LGTM
question: could we replace 4 with a constant defined in target.h? Can it be calculated from:
#define RBM_BYTE_REGS (RBM_EAX|RBM_ECX|RBM_EDX|RBM_EBX)
#define RBM_NON_BYTE_REGS (RBM_ESI|RBM_EDI)
?
Also, side question, could you please clarify for me why RBM_NON_BYTE_REGS does not include ESP, EBP? It has only one use but could it be wrong?
I believe it's because we don't use |
|
/backport to release/5.0 |
|
Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/233483644 |
ghost
locked as resolved and limited conversation to collaborators
Fix #40963