Skip to content

Commit

Permalink
Mark promoted SIMD locals used by HWIs as DNER (#64855)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
SingleAccretion committed Feb 6, 2022
1 parent db1bbf4 commit 207f2b6
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 8 deletions.
5 changes: 5 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9893,6 +9893,10 @@ void Compiler::EnregisterStats::RecordLocal(const LclVarDsc* varDsc)
m_returnSpCheck++;
break;

case DoNotEnregisterReason::SimdUserForcesDep:
m_simdUserForcesDep++;
break;

default:
unreached();
break;
Expand Down Expand Up @@ -10014,6 +10018,7 @@ void Compiler::EnregisterStats::Dump(FILE* fout) const
PRINT_STATS(m_swizzleArg, notEnreg);
PRINT_STATS(m_blockOpRet, notEnreg);
PRINT_STATS(m_returnSpCheck, notEnreg);
PRINT_STATS(m_simdUserForcesDep, notEnreg);

fprintf(fout, "\nAddr exposed details:\n");
if (m_addrExposed == 0)
Expand Down
12 changes: 7 additions & 5 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -385,11 +385,12 @@ enum class DoNotEnregisterReason
#endif
LclAddrNode, // the local is accessed with LCL_ADDR_VAR/FLD.
CastTakesAddr,
StoreBlkSrc, // the local is used as STORE_BLK source.
OneAsgRetyping, // fgMorphOneAsgBlockOp prevents this local from being enregister.
SwizzleArg, // the local is passed using LCL_FLD as another type.
BlockOpRet, // the struct is returned and it promoted or there is a cast.
ReturnSpCheck // the local is used to do SP check
StoreBlkSrc, // the local is used as STORE_BLK source.
OneAsgRetyping, // fgMorphOneAsgBlockOp prevents this local from being enregister.
SwizzleArg, // the local is passed using LCL_FLD as another type.
BlockOpRet, // the struct is returned and it promoted or there is a cast.
ReturnSpCheck, // the local is used to do SP check
SimdUserForcesDep // a promoted struct was used by a SIMD/HWI node; it must be dependently promoted
};

enum class AddressExposedReason
Expand Down Expand Up @@ -10507,6 +10508,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
unsigned m_swizzleArg;
unsigned m_blockOpRet;
unsigned m_returnSpCheck;
unsigned m_simdUserForcesDep;
unsigned m_liveInOutHndlr;
unsigned m_depField;
unsigned m_noRegVars;
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2584,6 +2584,10 @@ void Compiler::lvaSetVarDoNotEnregister(unsigned varNum DEBUGARG(DoNotEnregister
JITDUMP("Used for SP check\n");
break;

case DoNotEnregisterReason::SimdUserForcesDep:
JITDUMP("Promoted struct used by a SIMD/HWI node\n");
break;

default:
unreached();
break;
Expand Down
21 changes: 18 additions & 3 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14287,11 +14287,26 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp)
for (GenTree** use : multiOp->UseEdges())
{
*use = fgMorphTree(*use);
multiOp->gtFlags |= ((*use)->gtFlags & GTF_ALL_EFFECT);

if (dontCseConstArguments && (*use)->OperIsConst())
GenTree* operand = *use;
multiOp->gtFlags |= (operand->gtFlags & GTF_ALL_EFFECT);

if (dontCseConstArguments && operand->OperIsConst())
{
operand->SetDoNotCSE();
}

// Promoted structs after morph must be in one of two states:
// a) Fully eliminated from the IR (independent promotion) OR only be
// used by "special" nodes (e. g. LHS of ASGs for multi-reg structs).
// b) Marked as do-not-enregister (dependent promotion).
//
// So here we preserve this invariant and mark any promoted structs as do-not-enreg.
//
if (operand->OperIs(GT_LCL_VAR) && lvaGetDesc(operand->AsLclVar())->lvPromoted)
{
(*use)->SetDoNotCSE();
lvaSetVarDoNotEnregister(operand->AsLclVar()->GetLclNum()
DEBUGARG(DoNotEnregisterReason::SimdUserForcesDep));
}
}

Expand Down

0 comments on commit 207f2b6

Please sign in to comment.