Skip to content

Commit 207f2b6

Browse files
Mark promoted SIMD locals used by HWIs as DNER (#64855)
As the added comment states, we must do this to get dependent promotion, as otherwise the compiler does not support independent promotion of structs not fully eliminated from the IR, save some special cases (such as multi-reg structs). We will only need to do this very rarely, as otherwise we mark SIMD locals used by SIMDs/HWIs specially when creating those nodes so as not to promote them. This issue was revealed by forward substituion, where we had: SIMD a = OBJ(ADDR(b)) // b is SIMD too HWI(a) And forward-substituted the promoted "b" into "HWI". Notably, by the time we are forward-substituting, "we cannot really do anything about it", as struct promotion has already run, and by the time we get to morph, we too cannot do much save some gymnastics with conjuring up a tree of inserts from individual fields.
1 parent db1bbf4 commit 207f2b6

4 files changed

Lines changed: 34 additions & 8 deletions

File tree

src/coreclr/jit/compiler.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9893,6 +9893,10 @@ void Compiler::EnregisterStats::RecordLocal(const LclVarDsc* varDsc)
98939893
m_returnSpCheck++;
98949894
break;
98959895

9896+
case DoNotEnregisterReason::SimdUserForcesDep:
9897+
m_simdUserForcesDep++;
9898+
break;
9899+
98969900
default:
98979901
unreached();
98989902
break;
@@ -10014,6 +10018,7 @@ void Compiler::EnregisterStats::Dump(FILE* fout) const
1001410018
PRINT_STATS(m_swizzleArg, notEnreg);
1001510019
PRINT_STATS(m_blockOpRet, notEnreg);
1001610020
PRINT_STATS(m_returnSpCheck, notEnreg);
10021+
PRINT_STATS(m_simdUserForcesDep, notEnreg);
1001710022

1001810023
fprintf(fout, "\nAddr exposed details:\n");
1001910024
if (m_addrExposed == 0)

src/coreclr/jit/compiler.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -385,11 +385,12 @@ enum class DoNotEnregisterReason
385385
#endif
386386
LclAddrNode, // the local is accessed with LCL_ADDR_VAR/FLD.
387387
CastTakesAddr,
388-
StoreBlkSrc, // the local is used as STORE_BLK source.
389-
OneAsgRetyping, // fgMorphOneAsgBlockOp prevents this local from being enregister.
390-
SwizzleArg, // the local is passed using LCL_FLD as another type.
391-
BlockOpRet, // the struct is returned and it promoted or there is a cast.
392-
ReturnSpCheck // the local is used to do SP check
388+
StoreBlkSrc, // the local is used as STORE_BLK source.
389+
OneAsgRetyping, // fgMorphOneAsgBlockOp prevents this local from being enregister.
390+
SwizzleArg, // the local is passed using LCL_FLD as another type.
391+
BlockOpRet, // the struct is returned and it promoted or there is a cast.
392+
ReturnSpCheck, // the local is used to do SP check
393+
SimdUserForcesDep // a promoted struct was used by a SIMD/HWI node; it must be dependently promoted
393394
};
394395

395396
enum class AddressExposedReason
@@ -10507,6 +10508,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
1050710508
unsigned m_swizzleArg;
1050810509
unsigned m_blockOpRet;
1050910510
unsigned m_returnSpCheck;
10511+
unsigned m_simdUserForcesDep;
1051010512
unsigned m_liveInOutHndlr;
1051110513
unsigned m_depField;
1051210514
unsigned m_noRegVars;

src/coreclr/jit/lclvars.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2584,6 +2584,10 @@ void Compiler::lvaSetVarDoNotEnregister(unsigned varNum DEBUGARG(DoNotEnregister
25842584
JITDUMP("Used for SP check\n");
25852585
break;
25862586

2587+
case DoNotEnregisterReason::SimdUserForcesDep:
2588+
JITDUMP("Promoted struct used by a SIMD/HWI node\n");
2589+
break;
2590+
25872591
default:
25882592
unreached();
25892593
break;

src/coreclr/jit/morph.cpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14287,11 +14287,26 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp)
1428714287
for (GenTree** use : multiOp->UseEdges())
1428814288
{
1428914289
*use = fgMorphTree(*use);
14290-
multiOp->gtFlags |= ((*use)->gtFlags & GTF_ALL_EFFECT);
1429114290

14292-
if (dontCseConstArguments && (*use)->OperIsConst())
14291+
GenTree* operand = *use;
14292+
multiOp->gtFlags |= (operand->gtFlags & GTF_ALL_EFFECT);
14293+
14294+
if (dontCseConstArguments && operand->OperIsConst())
14295+
{
14296+
operand->SetDoNotCSE();
14297+
}
14298+
14299+
// Promoted structs after morph must be in one of two states:
14300+
// a) Fully eliminated from the IR (independent promotion) OR only be
14301+
// used by "special" nodes (e. g. LHS of ASGs for multi-reg structs).
14302+
// b) Marked as do-not-enregister (dependent promotion).
14303+
//
14304+
// So here we preserve this invariant and mark any promoted structs as do-not-enreg.
14305+
//
14306+
if (operand->OperIs(GT_LCL_VAR) && lvaGetDesc(operand->AsLclVar())->lvPromoted)
1429314307
{
14294-
(*use)->SetDoNotCSE();
14308+
lvaSetVarDoNotEnregister(operand->AsLclVar()->GetLclNum()
14309+
DEBUGARG(DoNotEnregisterReason::SimdUserForcesDep));
1429514310
}
1429614311
}
1429714312

0 commit comments

Comments
 (0)