Skip to content

Commit d52b303

Browse files
jakobbotschjakobbotsch
authored
JIT: Propagate multi-reg-index for upper-vector-restore ref positions (#88380)
`LinearScan::resolveRegisters` will write the register back to the IR node for an upper-vector-restore RP, so without this propagation we would overwrite the register assignment for an unrelated field. For example, for IR like ```scala N001 ( 9, 6) [000090] m------N--- t90 = LCL_VAR struct<JIT.HardwareIntrinsics.Arm._AdvSimd.SimpleTernaryOpTest__AbsoluteDifferenceWideningUpperAndAdd_Vector128_UInt32+TestStruct, 48>(P) V00 loc0 ▌ simd16 field V00._fld1 (fldOffset=0x0) -> V14 tmp10 (last use) ▌ simd16 field V00._fld2 (fldOffset=0x10) -> V15 tmp11 (last use) ▌ simd16 field V00._fld3 (fldOffset=0x20) -> V16 tmp12 (last use) $540 ┌──▌ t90 struct N002 ( 10, 7) [000091] ----------- ▌ RETURN struct $VN.Void ``` we could build ref positions such as ``` [000091] 449.#639 U14 UVRs UVRes NA │ │ │ │ │ │ │ │ │ │ │ │ │V16a│ │ │ │ 449.#640 d0 Fixd Keep d0 │ │ │ │ │ │ │ │ │ │ │ │ │V16a│ │ │ │ 449.#641 V14 Use * ReLod d0 │ │ │ │ │ │ │ │ │ │ │V14a│ │V16a│ │ │ │ Keep d0 │ │ │ │ │ │ │ │ │ │ │V14i│ │V16a│ │ │ │ 449.#642 U15 UVRs UVRes NA │ │ │ │ │ │ │ │ │ │ │ │ │V16a│ │ │ │ 449.#643 d1 Fixd Keep d1 │ │ │ │ │ │ │ │ │ │ │ │ │V16a│ │ │ │ 449.#644 V15 Use * ReLod d1 │ │ │ │ │ │ │ │ │ │ │ │V15a│V16a│ │ │ │ Keep d1 │ │ │ │ │ │ │ │ │ │ │ │V15i│V16a│ │ │ │ 449.#645 d2 Fixd Keep d2 │ │ │ │ │ │ │ │ │ │ │ │ │V16a│ │ │ │ 449.#646 V16 Use * Keep d2 │ │ │ │ │ │ │ │ │ │ │ │ │V16i│ │ │ │ ``` When writing back register assignments the upper-vector-restore at #642 ended up overwriting the assignment for the first field on [000090], resulting in ```scala N447 ( 9, 6) [000090] m------N--z t90 = LCL_VAR struct<JIT.HardwareIntrinsics.Arm._AdvSimd.SimpleTernaryOpTest__AbsoluteDifferenceWideningUpperAndAdd_Vector128_UInt32+TestStruct, 48>(P) V00 loc0 NA ▌ simd16 field V00._fld1 (fldOffset=0x0) -> V14 tmp10 (last use) ▌ simd16 field V00._fld2 (fldOffset=0x10) -> V15 tmp11 (last use) ▌ simd16 field V00._fld3 (fldOffset=0x20) -> V16 tmp12 d2 (last use) REG NA,d1,d2 $540 ┌──▌ t90 struct N449 ( 10, 7) [000091] ----------- ▌ RETURN struct REG NA $VN.Void ``` (note the REG NA instead of REG d0).
1 parent 5e1608d commit d52b303

File tree

2 files changed

+10
-11
lines changed

2 files changed

+10
-11
lines changed

‎src/coreclr/jit/lsra.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,10 +1045,8 @@ class LinearScan : public LinearScanInterface
10451045

10461046
#if FEATURE_PARTIAL_SIMD_CALLEE_SAVE
10471047
void buildUpperVectorSaveRefPositions(GenTree* tree, LsraLocation currentLoc, regMaskTP fpCalleeKillSet);
1048-
void buildUpperVectorRestoreRefPosition(Interval* lclVarInterval,
1049-
LsraLocation currentLoc,
1050-
GenTree* node,
1051-
bool isUse);
1048+
void buildUpperVectorRestoreRefPosition(
1049+
Interval* lclVarInterval, LsraLocation currentLoc, GenTree* node, bool isUse, unsigned multiRegIdx);
10521050
#endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE
10531051

10541052
#if defined(UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)

‎src/coreclr/jit/lsrabuild.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,11 +1596,10 @@ void LinearScan::buildUpperVectorSaveRefPositions(GenTree* tree, LsraLocation cu
15961596
// If null, the restore will be inserted at the end of the block.
15971597
// isUse - If the refPosition that is about to be created represents a use or not.
15981598
// - If not, it would be the one at the end of the block.
1599+
// multiRegIdx - Register position if this restore corresponds to a field of a multi reg node.
15991600
//
1600-
void LinearScan::buildUpperVectorRestoreRefPosition(Interval* lclVarInterval,
1601-
LsraLocation currentLoc,
1602-
GenTree* node,
1603-
bool isUse)
1601+
void LinearScan::buildUpperVectorRestoreRefPosition(
1602+
Interval* lclVarInterval, LsraLocation currentLoc, GenTree* node, bool isUse, unsigned multiRegIdx)
16041603
{
16051604
if (lclVarInterval->isPartiallySpilled)
16061605
{
@@ -1611,6 +1610,8 @@ void LinearScan::buildUpperVectorRestoreRefPosition(Interval* lclVarInterval,
16111610
newRefPosition(upperVectorInterval, currentLoc, RefTypeUpperVectorRestore, node, RBM_NONE);
16121611
lclVarInterval->isPartiallySpilled = false;
16131612

1613+
restorePos->setMultiRegIdx(multiRegIdx);
1614+
16141615
if (isUse)
16151616
{
16161617
// If there was a use of the restore before end of the block restore,
@@ -2536,7 +2537,7 @@ void LinearScan::buildIntervals()
25362537
while (largeVectorVarsIter.NextElem(&largeVectorVarIndex))
25372538
{
25382539
Interval* lclVarInterval = getIntervalForLocalVar(largeVectorVarIndex);
2539-
buildUpperVectorRestoreRefPosition(lclVarInterval, currentLoc, nullptr, false);
2540+
buildUpperVectorRestoreRefPosition(lclVarInterval, currentLoc, nullptr, false, 0);
25402541
}
25412542
#endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE
25422543

@@ -3195,7 +3196,7 @@ RefPosition* LinearScan::BuildUse(GenTree* operand, regMaskTP candidates, int mu
31953196
UpdatePreferencesOfDyingLocal(interval);
31963197
}
31973198
#if FEATURE_PARTIAL_SIMD_CALLEE_SAVE
3198-
buildUpperVectorRestoreRefPosition(interval, currentLoc, operand, true);
3199+
buildUpperVectorRestoreRefPosition(interval, currentLoc, operand, true, (unsigned)multiRegIdx);
31993200
#endif
32003201
}
32013202
else if (operand->IsMultiRegLclVar())
@@ -3209,7 +3210,7 @@ RefPosition* LinearScan::BuildUse(GenTree* operand, regMaskTP candidates, int mu
32093210
VarSetOps::RemoveElemD(compiler, currentLiveVars, fieldVarDsc->lvVarIndex);
32103211
}
32113212
#if FEATURE_PARTIAL_SIMD_CALLEE_SAVE
3212-
buildUpperVectorRestoreRefPosition(interval, currentLoc, operand, true);
3213+
buildUpperVectorRestoreRefPosition(interval, currentLoc, operand, true, (unsigned)multiRegIdx);
32133214
#endif
32143215
}
32153216
else

0 commit comments

Comments
 (0)