Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete the zero-offset field sequence map #71455

Merged
merged 13 commits into from
Jul 8, 2022
Merged

Delete the zero-offset field sequence map #71455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
85a0ab4
Delete the unused GTF_ICON_FIELD_OFF
SingleAccretion May 9, 2022
86f0e3a
Do not try to find "IND(struct ARR_ADDR)" handle
SingleAccretion Jun 14, 2022
4822795
Delete the zero-offset field sequence map
SingleAccretion May 9, 2022
2e9e7b7
Make all field sequences singletons
SingleAccretion May 9, 2022
3a9095e
Print struct field offsets in the sequence
SingleAccretion May 9, 2022
5c87956
Improve address mode marking to handle COMMAs
SingleAccretion Jun 14, 2022
f2c866e
Delete NotAField
SingleAccretion Jun 29, 2022
2159def
Refactor the field sequence store
SingleAccretion Jun 29, 2022
f5341c2
FieldSeqNode -> FieldSeq
SingleAccretion Jun 29, 2022
c760d47
Turn asserts into "noway_assert"s
SingleAccretion Jun 29, 2022
319febe
Fix formatting
SingleAccretion Jun 30, 2022
bdbaf25
Take down SPMI
SingleAccretion Jul 2, 2022
0cad61a
Resurrect SPMI to work around a CG2 bug
SingleAccretion Jul 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 10 additions & 38 deletions src/coreclr/jit/assertionprop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1111,11 +1111,6 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse
{
printf(".%02u", curAssertion->op1.lcl.ssaNum);
}
if (curAssertion->op2.zeroOffsetFieldSeq != nullptr)
{
printf(" Zero");
gtDispFieldSeq(curAssertion->op2.zeroOffsetFieldSeq);
}
break;

case O2K_CONST_INT:
Expand Down Expand Up @@ -1673,14 +1668,10 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
goto DONE_ASSERTION; // Don't make an assertion
}

FieldSeqNode* zeroOffsetFieldSeq = nullptr;
GetZeroOffsetFieldMap()->Lookup(op2, &zeroOffsetFieldSeq);

assertion.op2.kind = O2K_LCLVAR_COPY;
assertion.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair);
assertion.op2.lcl.lclNum = lclNum2;
assertion.op2.lcl.ssaNum = op2->AsLclVarCommon()->GetSsaNum();
assertion.op2.zeroOffsetFieldSeq = zeroOffsetFieldSeq;
assertion.op2.kind = O2K_LCLVAR_COPY;
assertion.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair);
assertion.op2.lcl.lclNum = lclNum2;
assertion.op2.lcl.ssaNum = op2->AsLclVarCommon()->GetSsaNum();

// Ok everything has been set and the assertion looks good
assertion.assertionKind = assertionKind;
Expand Down Expand Up @@ -3524,23 +3515,17 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion,
}

// Extract the matching lclNum and ssaNum, as well as the field sequence.
unsigned copyLclNum;
unsigned copySsaNum;
FieldSeqNode* zeroOffsetFieldSeq;
unsigned copyLclNum;
unsigned copySsaNum;
if (op1.lcl.lclNum == lclNum)
{
copyLclNum = op2.lcl.lclNum;
copySsaNum = op2.lcl.ssaNum;
zeroOffsetFieldSeq = op2.zeroOffsetFieldSeq;
copyLclNum = op2.lcl.lclNum;
copySsaNum = op2.lcl.ssaNum;
}
else
{
copyLclNum = op1.lcl.lclNum;
copySsaNum = op1.lcl.ssaNum;
zeroOffsetFieldSeq = nullptr; // Only the RHS of an assignment can have a FldSeq.
assert(optLocalAssertionProp); // Were we to perform replacements in global propagation, that makes copy
// assertions for control flow ("if (a == b) { ... }"), where both operands
// could have a FldSeq, we'd need to save it for "op1" too.
copyLclNum = op1.lcl.lclNum;
copySsaNum = op1.lcl.ssaNum;
}

if (!optLocalAssertionProp)
Expand Down Expand Up @@ -3572,19 +3557,6 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion,
tree->SetLclNum(copyLclNum);
tree->SetSsaNum(copySsaNum);

// The sequence we are propagating (if any) represents the inner fields.
if (zeroOffsetFieldSeq != nullptr)
{
FieldSeqNode* outerZeroOffsetFieldSeq = nullptr;
if (GetZeroOffsetFieldMap()->Lookup(tree, &outerZeroOffsetFieldSeq))
{
zeroOffsetFieldSeq = GetFieldSeqStore()->Append(zeroOffsetFieldSeq, outerZeroOffsetFieldSeq);
GetZeroOffsetFieldMap()->Remove(tree);
}

fgAddFieldSeqForZeroOffset(tree, zeroOffsetFieldSeq);
}

#ifdef DEBUG
if (verbose)
{
Expand Down
14 changes: 4 additions & 10 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1935,11 +1935,10 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
m_nodeTestData = nullptr;
m_loopHoistCSEClass = FIRST_LOOP_HOIST_CSE_CLASS;
#endif
m_switchDescMap = nullptr;
m_blockToEHPreds = nullptr;
m_fieldSeqStore = nullptr;
m_zeroOffsetFieldMap = nullptr;
m_refAnyClass = nullptr;
m_switchDescMap = nullptr;
m_blockToEHPreds = nullptr;
m_fieldSeqStore = nullptr;
m_refAnyClass = nullptr;
for (MemoryKind memoryKind : allMemoryKinds())
{
m_memorySsaMap[memoryKind] = nullptr;
Expand Down Expand Up @@ -9608,11 +9607,6 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
chars += printf("[GTF_ICON_STATIC_BOX_PTR]");
break;

case GTF_ICON_FIELD_OFF:

chars += printf("[ICON_FIELD_OFF]");
break;

default:
assert(!"a forgotten handle flag");
break;
Expand Down
68 changes: 11 additions & 57 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2266,7 +2266,7 @@ class Compiler
GenTree* op2 = nullptr);

GenTreeIntCon* gtNewIconNode(ssize_t value, var_types type = TYP_INT);
GenTreeIntCon* gtNewIconNode(unsigned fieldOffset, FieldSeqNode* fieldSeq);
GenTreeIntCon* gtNewIconNode(unsigned fieldOffset, FieldSeq* fieldSeq);
GenTreeIntCon* gtNewNull();
GenTreeIntCon* gtNewTrue();
GenTreeIntCon* gtNewFalse();
Expand All @@ -2277,7 +2277,7 @@ class Compiler

GenTree* gtNewIndOfIconHandleNode(var_types indType, size_t value, GenTreeFlags iconFlags, bool isInvariant);

GenTreeIntCon* gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeqNode* fields = nullptr);
GenTreeIntCon* gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeq* fields = nullptr);

GenTreeFlags gtTokenToIconFlags(unsigned token);

Expand Down Expand Up @@ -2834,7 +2834,6 @@ class Compiler
unsigned gtDispMultiRegCount(GenTree* tree);
#endif
void gtDispRegVal(GenTree* tree);
void gtDispZeroFieldSeq(GenTree* tree);
void gtDispVN(GenTree* tree);
void gtDispCommonEndLine(GenTree* tree);

Expand Down Expand Up @@ -2872,8 +2871,7 @@ class Compiler
void gtGetArgMsg(GenTreeCall* call, CallArg* arg, char* bufp, unsigned bufLength);
void gtGetLateArgMsg(GenTreeCall* call, CallArg* arg, char* bufp, unsigned bufLength);
void gtDispArgList(GenTreeCall* call, GenTree* lastCallOperand, IndentStack* indentStack);
void gtDispAnyFieldSeq(FieldSeqNode* fieldSeq);
void gtDispFieldSeq(FieldSeqNode* pfsn);
void gtDispFieldSeq(FieldSeq* fieldSeq, ssize_t offset);

void gtDispRange(LIR::ReadOnlyRange const& range);

Expand Down Expand Up @@ -4738,14 +4736,10 @@ class Compiler

void fgValueNumberArrayElemStore(GenTree* storeNode, VNFuncApp* addrFunc, unsigned storeSize, ValueNum value);

void fgValueNumberFieldLoad(GenTree* loadTree, GenTree* baseAddr, FieldSeqNode* fieldSeq, ssize_t offset);
void fgValueNumberFieldLoad(GenTree* loadTree, GenTree* baseAddr, FieldSeq* fieldSeq, ssize_t offset);

void fgValueNumberFieldStore(GenTree* storeNode,
GenTree* baseAddr,
FieldSeqNode* fieldSeq,
ssize_t offset,
unsigned storeSize,
ValueNum value);
void fgValueNumberFieldStore(
GenTree* storeNode, GenTree* baseAddr, FieldSeq* fieldSeq, ssize_t offset, unsigned storeSize, ValueNum value);

// Compute the value number for a byref-exposed load of the given type via the given pointerVN.
ValueNum fgValueNumberByrefExposedLoad(var_types type, ValueNum pointerVN);
Expand Down Expand Up @@ -4857,7 +4851,6 @@ class Compiler

#ifdef DEBUG
void fgDebugCheckExceptionSets();
void fgDebugCheckValueNumberedTree(GenTree* tree);
#endif

// These are the current value number for the memory implicit variables while
Expand Down Expand Up @@ -7006,11 +6999,7 @@ class Compiler
GenTreeFlags iconFlags; // gtFlags
};
union {
struct
{
SsaVar lcl;
FieldSeqNode* zeroOffsetFieldSeq;
};
SsaVar lcl;
IntVal u1;
__int64 lconVal;
double dconVal;
Expand Down Expand Up @@ -7113,8 +7102,7 @@ class Compiler

case O2K_LCLVAR_COPY:
return (op2.lcl.lclNum == that->op2.lcl.lclNum) &&
(!vnBased || op2.lcl.ssaNum == that->op2.lcl.ssaNum) &&
(op2.zeroOffsetFieldSeq == that->op2.zeroOffsetFieldSeq);
(!vnBased || (op2.lcl.ssaNum == that->op2.lcl.ssaNum));

case O2K_SUBRANGE:
return op2.u2.Equals(that->op2.u2);
Expand Down Expand Up @@ -10415,54 +10403,20 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
void JitTestCheckVN(); // Value numbering tests.
#endif // DEBUG

// The "FieldSeqStore", for canonicalizing field sequences. See the definition of FieldSeqStore for
// operations.
FieldSeqStore* m_fieldSeqStore;

FieldSeqStore* GetFieldSeqStore()
{
Compiler* compRoot = impInlineRoot();
if (compRoot->m_fieldSeqStore == nullptr)
{
// Create a CompAllocator that labels sub-structure with CMK_FieldSeqStore, and use that for allocation.
CompAllocator ialloc(getAllocator(CMK_FieldSeqStore));
compRoot->m_fieldSeqStore = new (ialloc) FieldSeqStore(ialloc);
CompAllocator alloc = getAllocator(CMK_FieldSeqStore);
compRoot->m_fieldSeqStore = new (alloc) FieldSeqStore(alloc);
}
return compRoot->m_fieldSeqStore;
}

typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, FieldSeqNode*> NodeToFieldSeqMap;

// Some nodes of "TYP_BYREF" or "TYP_I_IMPL" actually represent the address of a field within a struct, but since
// the offset of the field is zero, there's no "GT_ADD" node. We normally attach a field sequence to the constant
// that is added, but what do we do when that constant is zero, and is thus not present? We use this mechanism to
// attach the field sequence directly to the address node.
NodeToFieldSeqMap* m_zeroOffsetFieldMap;

NodeToFieldSeqMap* GetZeroOffsetFieldMap()
{
// Don't need to worry about inlining here
if (m_zeroOffsetFieldMap == nullptr)
{
// Create a CompAllocator that labels sub-structure with CMK_ZeroOffsetFieldMap, and use that for
// allocation.
CompAllocator ialloc(getAllocator(CMK_ZeroOffsetFieldMap));
m_zeroOffsetFieldMap = new (ialloc) NodeToFieldSeqMap(ialloc);
}
return m_zeroOffsetFieldMap;
}

// Requires that "op1" is a node of type "TYP_BYREF" or "TYP_I_IMPL". We are dereferencing this with the fields in
// "fieldSeq", whose offsets are required all to be zero. Ensures that any field sequence annotation currently on
// "op1" or its components is augmented by appending "fieldSeq". In practice, if "op1" is a GT_LCL_FLD, it has
// a field sequence as a member; otherwise, it may be the addition of an a byref and a constant, where the const
// has a field sequence -- in this case "fieldSeq" is appended to that of the constant; otherwise, we
// record the field sequence using the ZeroOffsetFieldMap described above.
//
// One exception above is that "op1" is a node of type "TYP_REF" where "op1" is a GT_LCL_VAR.
// This happens when System.Object vtable pointer is a regular field at offset 0 in System.Private.CoreLib in
// NativeAOT. Such case is handled same as the default case.
void fgAddFieldSeqForZeroOffset(GenTree* op1, FieldSeqNode* fieldSeq);
typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, FieldSeq*> NodeToFieldSeqMap;

NodeToUnsignedMap* m_memorySsaMap[MemoryKindCount];

Expand Down
14 changes: 4 additions & 10 deletions src/coreclr/jit/compiler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,15 +902,9 @@ inline GenTree* Compiler::gtNewLargeOperNode(genTreeOps oper, var_types type, Ge
* that may need to be fixed up).
*/

inline GenTreeIntCon* Compiler::gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeqNode* fields)
inline GenTreeIntCon* Compiler::gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeq* fields)
{
assert((flags & (GTF_ICON_HDL_MASK | GTF_ICON_FIELD_OFF)) != 0);

// Interpret "fields == NULL" as "not a field."
if (fields == nullptr)
{
fields = FieldSeqStore::NotAField();
}
assert((flags & GTF_ICON_HDL_MASK) != 0);

GenTreeIntCon* node;
#if defined(LATE_DISASM)
Expand Down Expand Up @@ -1366,7 +1360,7 @@ inline void GenTree::SetOper(genTreeOps oper, ValueNumberUpdate vnUpdate)
switch (oper)
{
case GT_CNS_INT:
AsIntCon()->gtFieldSeq = FieldSeqStore::NotAField();
AsIntCon()->gtFieldSeq = nullptr;
INDEBUG(AsIntCon()->gtTargetHandle = 0);
break;
#if defined(TARGET_ARM)
Expand Down Expand Up @@ -1548,7 +1542,7 @@ void GenTree::BashToConst(T value, var_types type /* = TYP_UNDEF */)
}

AsIntCon()->SetIconValue(static_cast<ssize_t>(value));
AsIntCon()->gtFieldSeq = FieldSeqStore::NotAField();
AsIntCon()->gtFieldSeq = nullptr;
break;

#if !defined(TARGET_64BIT)
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/compmemkind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ CompMemKindMacro(FixedBitVect)
CompMemKindMacro(Generic)
CompMemKindMacro(LocalAddressVisitor)
CompMemKindMacro(FieldSeqStore)
CompMemKindMacro(ZeroOffsetFieldMap)
CompMemKindMacro(MemorySsaMap)
CompMemKindMacro(MemoryPhiArg)
CompMemKindMacro(CSE)
Expand Down
Loading