Skip to content

Commit 4aa4ca5

Browse files
authored
JIT: Disallow parameters passed in float regs from being enregistered in int regs (#99286)
This change is a follow-up to 928ff30. The logic in that change is only correct under the assumption that parameters passed in float regs are never homed in integer regs. However, we can enregister structs of fitting size in integer registers if they have only whole uses, so that could still happen for a struct passed in float registers. This is a very rare case because such enregistration only kicks in when the struct parameter also was not promoted. This change disallows the problematic enregistration.
1 parent bfd5729 commit 4aa4ca5

File tree

2 files changed

+23
-0
lines changed

2 files changed

+23
-0
lines changed

src/coreclr/jit/codegencommon.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3795,6 +3795,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
37953795

37963796
regNumber begRegNum = genMapRegArgNumToRegNum(begReg, destMemType);
37973797
GetEmitter()->emitIns_Mov(insCopy, size, xtraReg, begRegNum, /* canSkip */ false);
3798+
assert(!genIsValidIntReg(xtraReg) || !genIsValidFloatReg(begRegNum));
37983799

37993800
regSet.verifyRegUsed(xtraReg);
38003801

@@ -3809,6 +3810,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
38093810
regNumber srcRegNum = genMapRegArgNumToRegNum(srcReg, destMemType);
38103811

38113812
GetEmitter()->emitIns_Mov(insCopy, size, destRegNum, srcRegNum, /* canSkip */ false);
3813+
assert(!genIsValidIntReg(destRegNum) || !genIsValidFloatReg(srcRegNum));
38123814

38133815
regSet.verifyRegUsed(destRegNum);
38143816

@@ -3860,6 +3862,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
38603862
regNumber destRegNum = genMapRegArgNumToRegNum(destReg, destMemType);
38613863

38623864
GetEmitter()->emitIns_Mov(insCopy, size, destRegNum, xtraReg, /* canSkip */ false);
3865+
assert(!genIsValidIntReg(destRegNum) || !genIsValidFloatReg(xtraReg));
38633866

38643867
regSet.verifyRegUsed(destRegNum);
38653868
/* mark the beginning register as processed */
@@ -3934,6 +3937,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
39343937
// todo -- suppress self move
39353938
GetEmitter()->emitIns_R_R_I_I(INS_mov, EA_4BYTE, destRegNum, regNum,
39363939
regArgTab[currentArgNum].slot - 1, 0);
3940+
assert(!genIsValidIntReg(destRegNum) || !genIsValidFloatReg(regNum));
39373941
regArgTab[currentArgNum].processed = true;
39383942
regArgMaskLive &= ~genRegMask(regNum);
39393943
}
@@ -4107,6 +4111,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
41074111
}
41084112
#endif
41094113
inst_Mov(destMemType, destRegNum, regNum, /* canSkip */ false, size);
4114+
assert(!genIsValidIntReg(destRegNum) || !genIsValidFloatReg(regNum));
41104115
}
41114116

41124117
/* mark the argument as processed */
@@ -4132,6 +4137,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
41324137
// Emit a shufpd with a 0 immediate, which preserves the 0th element of the dest reg
41334138
// and moves the 0th element of the src reg into the 1st element of the dest reg.
41344139
GetEmitter()->emitIns_R_R_I(INS_shufpd, emitActualTypeSize(varRegType), destRegNum, nextRegNum, 0);
4140+
assert(!genIsValidIntReg(destRegNum) || !genIsValidFloatReg(nextRegNum));
41354141
// Set destRegNum to regNum so that we skip the setting of the register below,
41364142
// but mark argNum as processed and clear regNum from the live mask.
41374143
destRegNum = regNum;
@@ -4159,6 +4165,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
41594165
noway_assert(genIsValidFloatReg(nextRegNum));
41604166
noway_assert(genIsValidFloatReg(destRegNum));
41614167
GetEmitter()->emitIns_Mov(INS_mov, EA_8BYTE, destRegNum, nextRegNum, /* canSkip */ false);
4168+
assert(!genIsValidIntReg(destRegNum) || !genIsValidFloatReg(nextRegNum));
41624169
}
41634170
}
41644171
#if defined(TARGET_ARM64) && defined(FEATURE_SIMD)
@@ -4179,6 +4186,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
41794186
noway_assert(genIsValidFloatReg(nextRegNum));
41804187
noway_assert(genIsValidFloatReg(destRegNum));
41814188
GetEmitter()->emitIns_R_R_I_I(INS_mov, EA_4BYTE, destRegNum, nextRegNum, i, 0);
4189+
assert(!genIsValidIntReg(destRegNum) || !genIsValidFloatReg(nextRegNum));
41824190
}
41834191
}
41844192
#endif // defined(TARGET_ARM64) && defined(FEATURE_SIMD)

src/coreclr/jit/lsra.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1681,6 +1681,21 @@ bool LinearScan::isRegCandidate(LclVarDsc* varDsc)
16811681
return false;
16821682
}
16831683

1684+
// Avoid allocating parameters that are passed in float regs into integer
1685+
// registers. We currently home float registers before integer registers,
1686+
// so that kind of enregistration can trash integer registers containing
1687+
// other parameters.
1688+
// We assume that these cases will be homed to float registers if they are
1689+
// promoted.
1690+
// TODO-CQ: Combine integer and float register homing to handle these kinds
1691+
// of conflicts.
1692+
if ((varDsc->TypeGet() == TYP_STRUCT) && varDsc->lvIsRegArg && !varDsc->lvPromoted &&
1693+
varTypeUsesIntReg(varDsc->GetRegisterType()) && genIsValidFloatReg(varDsc->GetArgReg()))
1694+
{
1695+
compiler->lvaSetVarDoNotEnregister(lclNum DEBUGARG(DoNotEnregisterReason::IsStructArg));
1696+
return false;
1697+
}
1698+
16841699
// Are we not optimizing and we have exception handlers?
16851700
// if so mark all args and locals as volatile, so that they
16861701
// won't ever get enregistered.

0 commit comments

Comments
 (0)