Skip to content

Commit

Permalink
Add IR support for modeling multiple SSA definitions as one store (#7…
Browse files Browse the repository at this point in the history 
…6139)

* Change how we track use SSA numbers for partial defs

Store them in the SSA descriptor instead of a map.

Memory (x64): +0.18%
PIN    (x64): -0.04%

The memory regression is not small...

* SsaNumInfo support

* Support in SSA, VN and CopyProp

For now, just the "CanBeReplacedWithItsField" case.
This enables some nice simplifications, even as the
general case gets more complex.

Two quirks have been added to attain zero diffs.

* Support in dumping

* TP tuning

Gets us back 0.05% on the PIN counter. Hard to believe but true.

* More TP tuning

Another 0.025%.
  • Loading branch information
@SingleAccretion
SingleAccretion authored Oct 19, 2022
1 parent 888ad2b commit 2b61381
Show file tree
Hide file tree
Showing 14 changed files with 720 additions and 408 deletions.
11 changes: 5 additions & 6 deletions src/coreclr/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1947,7 +1947,7 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
#endif // DEBUG

vnStore = nullptr;
m_opAsgnVarDefSsaNums = nullptr;
m_outlinedCompositeSsaNums = nullptr;
m_nodeToLoopMemoryBlockMap = nullptr;
fgSsaPassesCompleted = 0;
fgVNPassesCompleted = 0;
Expand Down Expand Up @@ -5320,11 +5320,10 @@ void Compiler::ResetOptAnnotations()
assert(opts.optRepeat);
assert(JitConfig.JitOptRepeatCount() > 0);
fgResetForSsa();
vnStore = nullptr;
m_opAsgnVarDefSsaNums = nullptr;
m_blockToEHPreds = nullptr;
fgSsaPassesCompleted = 0;
fgVNPassesCompleted = 0;
vnStore = nullptr;
m_blockToEHPreds = nullptr;
fgSsaPassesCompleted = 0;
fgVNPassesCompleted = 0;

for (BasicBlock* const block : Blocks())
{
Expand Down
69 changes: 31 additions & 38 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,17 +211,19 @@ class LclSsaVarDsc
// TODO-Cleanup: In the case of uninitialized variables the block is set to nullptr by
// SsaBuilder and changed to fgFirstBB during value numbering. It would be useful to
// investigate and perhaps eliminate this rather unexpected behavior.
BasicBlock* m_block;
BasicBlock* m_block = nullptr;
// The GT_ASG node that generates the definition, or nullptr for definitions
// of uninitialized variables.
GenTreeOp* m_asg;
GenTreeOp* m_asg = nullptr;
// The SSA number associated with the previous definition for partial (GTF_USEASG) defs.
unsigned m_useDefSsaNum = SsaConfig::RESERVED_SSA_NUM;

public:
LclSsaVarDsc() : m_block(nullptr), m_asg(nullptr)
LclSsaVarDsc()
{
}

LclSsaVarDsc(BasicBlock* block) : m_block(block), m_asg(nullptr)
LclSsaVarDsc(BasicBlock* block) : m_block(block)
{
}

Expand Down Expand Up @@ -251,6 +253,16 @@ class LclSsaVarDsc
m_asg = asg;
}

unsigned GetUseDefSsaNum() const
{
return m_useDefSsaNum;
}

void SetUseDefSsaNum(unsigned ssaNum)
{
m_useDefSsaNum = ssaNum;
}

ValueNumPair m_vnPair;
};

Expand Down Expand Up @@ -341,7 +353,7 @@ class SsaDefArray
}

// Get a pointer to the SSA definition at the specified index.
T* GetSsaDefByIndex(unsigned index)
T* GetSsaDefByIndex(unsigned index) const
{
assert(index < m_count);
return &m_array[index];
Expand All @@ -354,7 +366,7 @@ class SsaDefArray
}

// Get a pointer to the SSA definition associated with the specified SSA number.
T* GetSsaDef(unsigned ssaNum)
T* GetSsaDef(unsigned ssaNum) const
{
assert(ssaNum != SsaConfig::RESERVED_SSA_NUM);
return GetSsaDefByIndex(ssaNum - GetMinSsaNum());
Expand Down Expand Up @@ -1098,7 +1110,7 @@ class LclVarDsc
// Returns the address of the per-Ssa data for the given ssaNum (which is required
// not to be the SsaConfig::RESERVED_SSA_NUM, which indicates that the variable is
// not an SSA variable).
LclSsaVarDsc* GetPerSsaData(unsigned ssaNum)
LclSsaVarDsc* GetPerSsaData(unsigned ssaNum) const
{
return lvPerSsaData.GetSsaDef(ssaNum);
}
Expand Down Expand Up @@ -2758,6 +2770,9 @@ class Compiler
// the given "fldHnd", is such an object pointer.
bool gtIsStaticFieldPtrToBoxedStruct(var_types fieldNodeType, CORINFO_FIELD_HANDLE fldHnd);

bool gtStoreDefinesField(
LclVarDsc* fieldVarDsc, ssize_t offset, unsigned size, ssize_t* pFieldStoreOffset, unsigned* pFileStoreSize);

// Return true if call is a recursive call; return false otherwise.
// Note when inlining, this looks for calls back to the root method.
bool gtIsRecursiveCall(GenTreeCall* call)
Expand Down Expand Up @@ -2859,6 +2874,7 @@ class Compiler
char* gtGetLclVarName(unsigned lclNum);
void gtDispLclVar(unsigned lclNum, bool padForBiggestDisp = true);
void gtDispLclVarStructType(unsigned lclNum);
void gtDispSsaName(unsigned lclNum, unsigned ssaNum, bool isDef);
void gtDispClassLayout(ClassLayout* layout, var_types type);
void gtDispILLocation(const ILLocation& loc);
void gtDispStmt(Statement* stmt, const char* msg = nullptr);
Expand Down Expand Up @@ -3427,10 +3443,9 @@ class Compiler
bool lvaTempsHaveLargerOffsetThanVars();

// Returns "true" iff local variable "lclNum" is in SSA form.
bool lvaInSsa(unsigned lclNum)
bool lvaInSsa(unsigned lclNum) const
{
assert(lclNum < lvaCount);
return lvaTable[lclNum].lvInSsa;
return lvaGetDesc(lclNum)->lvInSsa;
}

unsigned lvaStubArgumentVar; // variable representing the secret stub argument coming in EAX
Expand Down Expand Up @@ -4691,20 +4706,9 @@ class Compiler
return BasicBlockRangeList(startBlock, endBlock);
}

// The presence of a partial definition presents some difficulties for SSA: this is both a use of some SSA name
// of "x", and a def of a new SSA name for "x". The tree only has one local variable for "x", so it has to choose
// whether to treat that as the use or def. It chooses the "use", and thus the old SSA name. This map allows us
// to record/recover the "def" SSA number, given the lcl var node for "x" in such a tree.
typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, unsigned> NodeToUnsignedMap;
NodeToUnsignedMap* m_opAsgnVarDefSsaNums;
NodeToUnsignedMap* GetOpAsgnVarDefSsaNums()
{
if (m_opAsgnVarDefSsaNums == nullptr)
{
m_opAsgnVarDefSsaNums = new (getAllocator()) NodeToUnsignedMap(getAllocator());
}
return m_opAsgnVarDefSsaNums;
}
// This array, managed by the SSA numbering infrastructure, keeps "outlined composite SSA numbers".
// See "SsaNumInfo::GetNum" for more details on when this is needed.
JitExpandArrayStack<unsigned>* m_outlinedCompositeSsaNums;

// This map tracks nodes whose value numbers explicitly or implicitly depend on memory states.
// The map provides the entry block of the most closely enclosing loop that
Expand Down Expand Up @@ -4733,12 +4737,6 @@ class Compiler
void optRecordLoopMemoryDependence(GenTree* tree, BasicBlock* block, ValueNum memoryVN);
void optCopyLoopMemoryDependence(GenTree* fromTree, GenTree* toTree);

// Requires that "lcl" has the GTF_VAR_DEF flag set. Returns the SSA number of "lcl".
// Except: assumes that lcl is a def, and if it is
// a partial def (GTF_VAR_USEASG), looks up and returns the SSA number for the "def",
// rather than the "use" SSA number recorded in the tree "lcl".
inline unsigned GetSsaNumForLocalVarDef(GenTree* lcl);

inline bool PreciseRefCountsRequired();

// Performs SSA conversion.
Expand Down Expand Up @@ -6754,11 +6752,7 @@ class Compiler
bool optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName);
void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName);
bool optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName);
void optCopyPropPushDef(GenTree* defNode,
GenTreeLclVarCommon* lclNode,
unsigned lclNum,
LclNumToLiveDefsMap* curSsaName);
unsigned optIsSsaLocal(GenTreeLclVarCommon* lclNode);
void optCopyPropPushDef(GenTree* defNode, GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName);
int optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2);
PhaseStatus optVnCopyProp();
INDEBUG(void optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName));
Expand Down Expand Up @@ -9483,7 +9477,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
STRESS_MODE(DO_WHILE_LOOPS) \
STRESS_MODE(MIN_OPTS) \
STRESS_MODE(REVERSE_FLAG) /* Will set GTF_REVERSE_OPS whenever we can */ \
STRESS_MODE(REVERSE_COMMA) /* Will reverse commas created with gtNewCommaNode */ \
STRESS_MODE(TAILCALL) /* Will make the call as a tailcall whenever legal */ \
STRESS_MODE(CATCH_ARG) /* Will spill catch arg */ \
STRESS_MODE(UNSAFE_BUFFER_CHECKS) \
Expand All @@ -9496,6 +9489,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
STRESS_MODE(BYREF_PROMOTION) /* Change undoPromotion decisions for byrefs */ \
STRESS_MODE(PROMOTE_FEWER_STRUCTS)/* Don't promote some structs that can be promoted */ \
STRESS_MODE(VN_BUDGET)/* Randomize the VN budget */ \
STRESS_MODE(SSA_INFO) /* Select lower thresholds for "complex" SSA num encoding */ \
\
/* After COUNT_VARN, stress level 2 does all of these all the time */ \
\
Expand All @@ -9508,7 +9502,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
STRESS_MODE(CHK_FLOW_UPDATE) \
STRESS_MODE(EMITTER) \
STRESS_MODE(CHK_REIMPORT) \
STRESS_MODE(FLATFP) \
STRESS_MODE(GENERIC_CHECK) \
STRESS_MODE(COUNT)

Expand Down Expand Up @@ -10465,7 +10458,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
return compRoot->m_fieldSeqStore;
}

typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, FieldSeq*> NodeToFieldSeqMap;
typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, unsigned> NodeToUnsignedMap;

NodeToUnsignedMap* m_memorySsaMap[MemoryKindCount];

Expand Down
30 changes: 0 additions & 30 deletions src/coreclr/jit/compiler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4082,36 +4082,6 @@ bool Compiler::fgVarNeedsExplicitZeroInit(unsigned varNum, bool bbInALoop, bool
return !info.compInitMem || (varDsc->lvIsTemp && !varDsc->HasGCPtr());
}

/*****************************************************************************/
unsigned Compiler::GetSsaNumForLocalVarDef(GenTree* lcl)
{
// Address-taken variables don't have SSA numbers.
if (!lvaInSsa(lcl->AsLclVarCommon()->GetLclNum()))
{
return SsaConfig::RESERVED_SSA_NUM;
}

if (lcl->gtFlags & GTF_VAR_USEASG)
{
// It's partial definition of a struct. "lcl" is both used and defined here;
// we've chosen in this case to annotate "lcl" with the SSA number (and VN) of the use,
// and to store the SSA number of the def in a side table.
unsigned ssaNum;
// In case of a remorph (fgMorph) in CSE/AssertionProp after SSA phase, there
// wouldn't be an entry for the USEASG portion of the indir addr, return
// reserved.
if (!GetOpAsgnVarDefSsaNums()->Lookup(lcl, &ssaNum))
{
return SsaConfig::RESERVED_SSA_NUM;
}
return ssaNum;
}
else
{
return lcl->AsLclVarCommon()->GetSsaNum();
}
}

inline bool Compiler::PreciseRefCountsRequired()
{
return opts.OptimizationEnabled();
Expand Down
Loading

0 comments on commit 2b61381

Please sign in to comment.