From d381c858f6426d5c1fea3a81544b4b58f36f4388 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Fri, 17 Nov 2023 18:25:27 -0800 Subject: [PATCH 1/4] 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. --- src/coreclr/jit/lsra.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 306971171052b2..040ed1ae3f5e0c 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -12457,6 +12457,14 @@ 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 register for which the odd half is also available. + if (currentInterval->registerType == TYP_DOUBLE) + { + candidates &= ~(busyRegs >> 1); + } +#endif // TARGET_ARM + #ifdef DEBUG inUseOrBusyRegsMask |= busyRegs; #endif From 6d8545db7d0a2d2f060b2431ab659797a7a246ee Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Tue, 21 Nov 2023 14:23:39 -0800 Subject: [PATCH 2/4] Updated comment to be more descriptive. --- src/coreclr/jit/lsra.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 040ed1ae3f5e0c..5439b950bc9175 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -12458,7 +12458,14 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval, candidates &= ~busyRegs; #ifdef TARGET_ARM - // For TYP_DOUBLE on ARM, we can only use register for which the odd half is also available. + // 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 + // `~(busyRegs >> 1)` clause below creates a mask to do this. Note one oddity: if busyRegs contained + // f0, we would mask off some general-purpose register. If it contains any general-purpose registers, + // the mask will also contain general-purpose registers. That's ok because candidates for a double + // type starts off as only the even floating-point registers, so masking off anything else is a no-op. if (currentInterval->registerType == TYP_DOUBLE) { candidates &= ~(busyRegs >> 1); From 445f131af0be41fd16b7f9592de73d486cb2a0b5 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Tue, 21 Nov 2023 16:52:44 -0800 Subject: [PATCH 3/4] Add new RBM_ALLDOUBLE_HIGH mask for referring to high odd register part of ARM double registers even/odd pairs. --- src/coreclr/jit/codegencommon.cpp | 2 +- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/lsra.cpp | 12 +++++------- src/coreclr/jit/targetarm.cpp | 2 ++ src/coreclr/jit/targetarm.h | 6 +++++- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 225700208b4a2d..54ef2f6bda303b 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4062,7 +4062,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 diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 3a25901f658411..12fba8ae137aff 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -3250,7 +3250,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 diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 5439b950bc9175..b02d935eeda033 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -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 @@ -12462,13 +12463,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval, // 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 - // `~(busyRegs >> 1)` clause below creates a mask to do this. Note one oddity: if busyRegs contained - // f0, we would mask off some general-purpose register. If it contains any general-purpose registers, - // the mask will also contain general-purpose registers. That's ok because candidates for a double - // type starts off as only the even floating-point registers, so masking off anything else is a no-op. + // clause below creates a mask to do this. if (currentInterval->registerType == TYP_DOUBLE) { - candidates &= ~(busyRegs >> 1); + candidates &= ~((busyRegs & RBM_ALLDOUBLE_HIGH) >> 1); } #endif // TARGET_ARM diff --git a/src/coreclr/jit/targetarm.cpp b/src/coreclr/jit/targetarm.cpp index dbb986a0e05b0e..8e117bae810270 100644 --- a/src/coreclr/jit/targetarm.cpp +++ b/src/coreclr/jit/targetarm.cpp @@ -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 diff --git a/src/coreclr/jit/targetarm.h b/src/coreclr/jit/targetarm.h index 3fa00cacbf4700..3be0c47cbd1951 100644 --- a/src/coreclr/jit/targetarm.h +++ b/src/coreclr/jit/targetarm.h @@ -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 @@ -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]; From ef25ee6688f9bd5941b5ed43bcd3912b9deaa5cd Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Wed, 22 Nov 2023 13:43:53 -0800 Subject: [PATCH 4/4] Fix disasm of arm32 double registers d10-d15 --- src/coreclr/jit/emitarm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/emitarm.cpp b/src/coreclr/jit/emitarm.cpp index 33ae40ee208ef5..270b26c4a80579 100644 --- a/src/coreclr/jit/emitarm.cpp +++ b/src/coreclr/jit/emitarm.cpp @@ -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