Skip to content

Commit

Permalink
Fix LSRA handling of arm32 double registers and spilling (dotnet#94947)
Browse files Browse the repository at this point in the history
* Fix LSRA handling of arm32 double registers and spilling

If LSRA uses `regsBusyUntilKill` to eliminate registers from consideration
for spilling, for spilling arm32 double type registers, it needs to
remove the even registers from the candidates. For example, if
`regsBusyUntilKill` contains `f3`, candidates also needs to remove `f2`,
since double register candidates only contains the even registers to
represent to pair of registers for an arm32 double.

* Updated comment to be more descriptive.

* Add new RBM_ALLDOUBLE_HIGH mask for referring to high odd
register part of ARM double registers even/odd pairs.

* Fix disasm of arm32 double registers d10-d15
  • Loading branch information
BruceForstall authored Nov 26, 2023
1 parent 30ef449 commit 63e07f5
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4025,7 +4025,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
// Don't process the double until both halves of the destination are clear.
if (genActualType(destMemType) == TYP_DOUBLE)
{
assert((destMask & RBM_DBL_REGS) != 0);
assert((destMask & RBM_ALLDOUBLE) != 0);
destMask |= genRegMask(REG_NEXT(destRegNum));
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3290,7 +3290,7 @@ __forceinline regMaskTP genMapArgNumToRegMask(unsigned argNum, var_types type)
#ifdef TARGET_ARM
if (type == TYP_DOUBLE)
{
assert((result & RBM_DBL_REGS) != 0);
assert((result & RBM_ALLDOUBLE) != 0);
result |= (result << 1);
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/emitarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6934,7 +6934,7 @@ void emitter::emitDispReg(regNumber reg, emitAttr attr, bool addComma)
else
{
assert(regIndex < 100);
printf("d%c%c", (regIndex / 10), (regIndex % 10));
printf("d%c%c", (regIndex / 10) + '0', (regIndex % 10) + '0');
}
}
else
Expand Down
17 changes: 15 additions & 2 deletions src/coreclr/jit/lsra.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7946,8 +7946,9 @@ regNumber LinearScan::getTempRegForResolution(BasicBlock* fromBlock,
#ifdef TARGET_ARM
if (type == TYP_DOUBLE)
{
// Exclude any doubles for which the odd half isn't in freeRegs.
freeRegs = freeRegs & ((freeRegs << 1) & RBM_ALLDOUBLE);
// Exclude any doubles for which the odd half isn't in freeRegs,
// and restrict down to just the even part of the even/odd pair.
freeRegs &= (freeRegs & RBM_ALLDOUBLE_HIGH) >> 1;
}
#endif

Expand Down Expand Up @@ -12457,6 +12458,18 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
candidates &= ~busyRegs;

#ifdef TARGET_ARM
// For TYP_DOUBLE on ARM, we can only use an even floating-point register for which the odd half
// is also available. Thus, for every busyReg that is an odd floating-point register, we need to
// remove from candidates the corresponding even floating-point register. For example, if busyRegs
// contains `f3`, we need to remove `f2` from the candidates for a double register interval. The
// clause below creates a mask to do this.
if (currentInterval->registerType == TYP_DOUBLE)
{
candidates &= ~((busyRegs & RBM_ALLDOUBLE_HIGH) >> 1);
}
#endif // TARGET_ARM

#ifdef DEBUG
inUseOrBusyRegsMask |= busyRegs;
#endif
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/targetarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@ const regNumber fltArgRegs [] = {REG_F0, REG_F1, REG_F2, REG_F3, REG_F4, REG_F5,
const regMaskTP fltArgMasks[] = {RBM_F0, RBM_F1, RBM_F2, RBM_F3, RBM_F4, RBM_F5, RBM_F6, RBM_F7, RBM_F8, RBM_F9, RBM_F10, RBM_F11, RBM_F12, RBM_F13, RBM_F14, RBM_F15 };
// clang-format on

static_assert_no_msg(RBM_ALLDOUBLE == (RBM_ALLDOUBLE_HIGH >> 1));

#endif // TARGET_ARM
6 changes: 5 additions & 1 deletion src/coreclr/jit/targetarm.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@
#define RBM_ALLFLOAT (RBM_FLT_CALLEE_SAVED | RBM_FLT_CALLEE_TRASH)
#define RBM_ALLDOUBLE (RBM_F0|RBM_F2|RBM_F4|RBM_F6|RBM_F8|RBM_F10|RBM_F12|RBM_F14|RBM_F16|RBM_F18|RBM_F20|RBM_F22|RBM_F24|RBM_F26|RBM_F28|RBM_F30)

// Double registers on ARM take two registers in odd/even pairs, e.g. f0 and f1, f2 and f3, etc. Mostly, we refer
// to double registers by their low, even, part. Sometimes we need to know that the high, odd, part is unused in
// a bitmask, say, hence we have this definition.
#define RBM_ALLDOUBLE_HIGH (RBM_F1|RBM_F3|RBM_F5|RBM_F7|RBM_F9|RBM_F11|RBM_F13|RBM_F15|RBM_F17|RBM_F19|RBM_F21|RBM_F23|RBM_F25|RBM_F27|RBM_F29|RBM_F31)

#define REG_VAR_ORDER REG_R3,REG_R2,REG_R1,REG_R0,REG_R4,REG_LR,REG_R12,\
REG_R5,REG_R6,REG_R7,REG_R8,REG_R9,REG_R10

Expand Down Expand Up @@ -281,7 +286,6 @@

#define RBM_ARG_REGS (RBM_ARG_0|RBM_ARG_1|RBM_ARG_2|RBM_ARG_3)
#define RBM_FLTARG_REGS (RBM_F0|RBM_F1|RBM_F2|RBM_F3|RBM_F4|RBM_F5|RBM_F6|RBM_F7|RBM_F8|RBM_F9|RBM_F10|RBM_F11|RBM_F12|RBM_F13|RBM_F14|RBM_F15)
#define RBM_DBL_REGS RBM_ALLDOUBLE

extern const regNumber fltArgRegs [MAX_FLOAT_REG_ARG];
extern const regMaskTP fltArgMasks[MAX_FLOAT_REG_ARG];
Expand Down

0 comments on commit 63e07f5

Please sign in to comment.