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.

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
48 changes: 10 additions & 38 deletions src/coreclr/jit/assertionprop.cpp
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
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
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
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
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