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

Only use first field maps for shared statics #66558

Merged
merged 7 commits into from
Mar 29, 2022
Merged

Only use first field maps for shared statics #66558

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b8bd75a
DEBUG-ONLY: FieldInfo (2.0)
SingleAccretion Dec 2, 2021
2036937
Use a handle for field sequences in VN
SingleAccretion Mar 12, 2022
d78929b
Encode the statics
SingleAccretion Mar 9, 2022
1e24123
Use shared-ness info in IsFieldAddr
SingleAccretion Mar 13, 2022
27c5d4f
Revert "DEBUG-ONLY: FieldInfo (2.0)"
SingleAccretion Mar 17, 2022
ba5f664
Revert unintentional assert change
SingleAccretion Mar 29, 2022
66c1d3b
Fix build
SingleAccretion Mar 29, 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
111 changes: 61 additions & 50 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10382,7 +10382,7 @@ void Compiler::gtDispFieldSeq(FieldSeqNode* pfsn)
while (pfsn != nullptr)
{
assert(pfsn != FieldSeqStore::NotAField()); // Can't exist in a field sequence list except alone
CORINFO_FIELD_HANDLE fldHnd = pfsn->m_fieldHnd;
CORINFO_FIELD_HANDLE fldHnd = pfsn->GetFieldHandleValue();
// First check the "pseudo" field handles...
if (fldHnd == FieldSeqStore::FirstElemPseudoField)
{
Expand All @@ -10396,7 +10396,7 @@ void Compiler::gtDispFieldSeq(FieldSeqNode* pfsn)
{
printf("%s", eeGetFieldName(fldHnd));
}
pfsn = pfsn->m_next;
pfsn = pfsn->GetNext();
if (pfsn != nullptr)
{
printf(", ");
Expand Down Expand Up @@ -16138,17 +16138,18 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF
fldSeq = AsOp()->gtOp2->AsIntCon()->gtFieldSeq;
baseAddr = AsOp()->gtOp1;
}

if (baseAddr != nullptr)
else
{
assert(!baseAddr->TypeIs(TYP_REF) || !comp->GetZeroOffsetFieldMap()->Lookup(baseAddr));
return false;
}

assert(!baseAddr->TypeIs(TYP_REF) || !comp->GetZeroOffsetFieldMap()->Lookup(baseAddr));
}
else if (IsCnsIntOrI() && IsIconHandle(GTF_ICON_STATIC_HDL))
{
assert(!comp->GetZeroOffsetFieldMap()->Lookup(this) && (AsIntCon()->gtFieldSeq != nullptr));
fldSeq = AsIntCon()->gtFieldSeq;
baseAddr = nullptr;
baseAddr = this;
}
else if (comp->GetZeroOffsetFieldMap()->Lookup(this, &fldSeq))
{
Expand All @@ -16159,7 +16160,7 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF
return false;
}

assert(fldSeq != nullptr);
assert((fldSeq != nullptr) && (baseAddr != nullptr));

if ((fldSeq == FieldSeqStore::NotAField()) || fldSeq->IsPseudoField())
{
Expand All @@ -16171,16 +16172,15 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF
// or a static field. To avoid the expense of calling "getFieldClass" here, we will instead
// rely on the invariant that TYP_REF base addresses can never appear for struct fields - we
// will effectively treat such cases ("possible" in unsafe code) as undefined behavior.
if (comp->eeIsFieldStatic(fldSeq->GetFieldHandle()))
if (fldSeq->IsStaticField())
{
// TODO-VNTypes: this code is out of sync w.r.t. boxed statics that are numbered with
// VNF_PtrToStatic and treated as "simple" while here we treat them as "complex".
// For shared statics, we must encode the logical instantiation argument.
if (fldSeq->IsSharedStaticField())
{
*pBaseAddr = baseAddr;
}

// TODO-VNTypes: we will always return the "baseAddr" here for now, but strictly speaking,
// we only need to do that if we have a shared field, to encode the logical "instantiation"
// argument. In all other cases, this serves no purpose and just leads to redundant maps.
*pBaseAddr = baseAddr;
*pFldSeq = fldSeq;
*pFldSeq = fldSeq;
return true;
}

Expand Down Expand Up @@ -16548,17 +16548,12 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree)
}
if (fieldSeq != nullptr)
{
while (fieldSeq->m_next != nullptr)
{
fieldSeq = fieldSeq->m_next;
}
fieldSeq = fieldSeq->GetTail();

if (fieldSeq != FieldSeqStore::NotAField() && !fieldSeq->IsPseudoField())
{
CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->m_fieldHnd;
CorInfoType fieldCorType = info.compCompHnd->getFieldType(fieldHnd, &structHnd);
// With unsafe code and type casts
// this can return a primitive type and have nullptr for structHnd
// see runtime/issues/38541
// Note we may have a primitive here (and correctly fail to obtain the handle)
eeGetFieldType(fieldSeq->GetFieldHandle(), &structHnd);
}
}
}
Expand Down Expand Up @@ -16830,6 +16825,8 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
}
else if (base->OperGet() == GT_ADD)
{
// TODO-VNTypes: use "IsFieldAddr" here instead.

// This could be a static field access.
//
// See if op1 is a static field base helper call
Expand All @@ -16845,20 +16842,15 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b

if (fieldSeq != nullptr)
{
while (fieldSeq->m_next != nullptr)
{
fieldSeq = fieldSeq->m_next;
}

assert(!fieldSeq->IsPseudoField());
fieldSeq = fieldSeq->GetTail();

// No benefit to calling gtGetFieldClassHandle here, as
// the exact field being accessed can vary.
CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->m_fieldHnd;
CORINFO_CLASS_HANDLE fieldClass = nullptr;
CorInfoType fieldCorType = info.compCompHnd->getFieldType(fieldHnd, &fieldClass);
CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->GetFieldHandle();
CORINFO_CLASS_HANDLE fieldClass = NO_CLASS_HANDLE;
var_types fieldType = eeGetFieldType(fieldHnd, &fieldClass);

assert(fieldCorType == CORINFO_TYPE_CLASS);
assert(fieldType == TYP_REF);
objClass = fieldClass;
}
}
Expand Down Expand Up @@ -17212,18 +17204,18 @@ void GenTree::ParseArrayAddress(
noway_assert(!"fldSeqIter is NotAField() in ParseArrayAddress");
}

if (!FieldSeqStore::IsPseudoField(fldSeqIter->m_fieldHnd))
if (!FieldSeqStore::IsPseudoField(fldSeqIter->GetFieldHandleValue()))
{
if (*pFldSeq == nullptr)
{
*pFldSeq = fldSeqIter;
}
CORINFO_CLASS_HANDLE fldCls = nullptr;
noway_assert(fldSeqIter->m_fieldHnd != nullptr);
CorInfoType cit = comp->info.compCompHnd->getFieldType(fldSeqIter->m_fieldHnd, &fldCls);
noway_assert(fldSeqIter->GetFieldHandle() != NO_CLASS_HANDLE);
SingleAccretion marked this conversation as resolved.
Show resolved Hide resolved
CorInfoType cit = comp->info.compCompHnd->getFieldType(fldSeqIter->GetFieldHandle(), &fldCls);
fieldOffsets += comp->compGetTypeSize(cit, fldCls);
}
fldSeqIter = fldSeqIter->m_next;
fldSeqIter = fldSeqIter->GetNext();
}

// Is there some portion of the "offset" beyond the first-elem offset and the struct field suffix we just computed?
Expand Down Expand Up @@ -17585,16 +17577,16 @@ void GenTree::LabelIndex(Compiler* comp, bool isConst)
// Note that the value of the below field doesn't matter; it exists only to provide a distinguished address.
//
// static
FieldSeqNode FieldSeqStore::s_notAField(nullptr, nullptr);
FieldSeqNode FieldSeqStore::s_notAField(nullptr, nullptr, FieldSeqNode::FieldKind::Instance);

// FieldSeqStore methods.
FieldSeqStore::FieldSeqStore(CompAllocator alloc) : m_alloc(alloc), m_canonMap(new (alloc) FieldSeqNodeCanonMap(alloc))
{
}

FieldSeqNode* FieldSeqStore::CreateSingleton(CORINFO_FIELD_HANDLE fieldHnd)
FieldSeqNode* FieldSeqStore::CreateSingleton(CORINFO_FIELD_HANDLE fieldHnd, FieldSeqNode::FieldKind fieldKind)
{
FieldSeqNode fsn(fieldHnd, nullptr);
FieldSeqNode fsn(fieldHnd, nullptr, fieldKind);
FieldSeqNode* res = nullptr;
if (m_canonMap->Lookup(fsn, &res))
{
Expand Down Expand Up @@ -17629,8 +17621,8 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b)
// Extremely special case for ConstantIndex pseudo-fields -- appending consecutive such
// together collapse to one.
}
else if (a->m_next == nullptr && a->m_fieldHnd == ConstantIndexPseudoField &&
b->m_fieldHnd == ConstantIndexPseudoField)
else if (a->GetNext() == nullptr && a->GetFieldHandleValue() == ConstantIndexPseudoField &&
b->GetFieldHandleValue() == ConstantIndexPseudoField)
{
return b;
}
Expand All @@ -17639,8 +17631,8 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b)
// We should never add a duplicate FieldSeqNode
assert(a != b);

FieldSeqNode* tmp = Append(a->m_next, b);
FieldSeqNode fsn(a->m_fieldHnd, tmp);
FieldSeqNode* tmp = Append(a->GetNext(), b);
FieldSeqNode fsn(a->GetFieldHandleValue(), tmp, a->GetKind());
FieldSeqNode* res = nullptr;
if (m_canonMap->Lookup(fsn, &res))
{
Expand All @@ -17665,19 +17657,38 @@ CORINFO_FIELD_HANDLE FieldSeqStore::FirstElemPseudoField =
CORINFO_FIELD_HANDLE FieldSeqStore::ConstantIndexPseudoField =
(CORINFO_FIELD_HANDLE)&FieldSeqStore::ConstantIndexPseudoFieldStruct;

bool FieldSeqNode::IsFirstElemFieldSeq()
FieldSeqNode::FieldSeqNode(CORINFO_FIELD_HANDLE fieldHnd, FieldSeqNode* next, FieldKind fieldKind) : m_next(next)
{
uintptr_t handleValue = reinterpret_cast<uintptr_t>(fieldHnd);

assert((handleValue & FIELD_KIND_MASK) == 0);
m_fieldHandleAndKind = handleValue | static_cast<uintptr_t>(fieldKind);

if (!FieldSeqStore::IsPseudoField(fieldHnd) && (fieldHnd != NO_FIELD_HANDLE))
{
assert(JitTls::GetCompiler()->eeIsFieldStatic(fieldHnd) == IsStaticField());
}
else
{
// Use the default for pseudo-fields.
assert(fieldKind == FieldKind::Instance);
}
}

bool FieldSeqNode::IsFirstElemFieldSeq() const
{
return m_fieldHnd == FieldSeqStore::FirstElemPseudoField;
return GetFieldHandleValue() == FieldSeqStore::FirstElemPseudoField;
}

bool FieldSeqNode::IsConstantIndexFieldSeq()
bool FieldSeqNode::IsConstantIndexFieldSeq() const
{
return m_fieldHnd == FieldSeqStore::ConstantIndexPseudoField;
return GetFieldHandleValue() == FieldSeqStore::ConstantIndexPseudoField;
}

bool FieldSeqNode::IsPseudoField() const
{
return m_fieldHnd == FieldSeqStore::FirstElemPseudoField || m_fieldHnd == FieldSeqStore::ConstantIndexPseudoField;
return (GetFieldHandleValue() == FieldSeqStore::FirstElemPseudoField) ||
(GetFieldHandleValue() == FieldSeqStore::ConstantIndexPseudoField);
}

#ifdef FEATURE_SIMD
Expand Down
70 changes: 54 additions & 16 deletions src/coreclr/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,36 +220,71 @@ class AssertionInfo
}
};

/*****************************************************************************/

// GT_FIELD nodes will be lowered into more "code-gen-able" representations, like
// GT_IND's of addresses, or GT_LCL_FLD nodes. We'd like to preserve the more abstract
// information, and will therefore annotate such lowered nodes with FieldSeq's. A FieldSeq
// represents a (possibly) empty sequence of fields. The fields are in the order
// in which they are dereferenced. The first field may be an object field or a struct field;
// all subsequent fields must be struct fields.
struct FieldSeqNode
class FieldSeqNode
{
CORINFO_FIELD_HANDLE m_fieldHnd;
FieldSeqNode* m_next;
public:
enum class FieldKind : uintptr_t
{
Instance = 0, // An instance field, object or struct.
SimpleStatic = 1, // Simple static field - the handle represents a unique location.
SharedStatic = 2, // Static field on a shared generic type: "Class<__Canon>.StaticField".
};

private:
static const uintptr_t FIELD_KIND_MASK = 0b11;

static_assert_no_msg(sizeof(CORINFO_FIELD_HANDLE) == sizeof(uintptr_t));

uintptr_t m_fieldHandleAndKind;
FieldSeqNode* m_next;

public:
FieldSeqNode(CORINFO_FIELD_HANDLE fieldHnd, FieldSeqNode* next, FieldKind fieldKind);

FieldKind GetKind() const
{
return static_cast<FieldKind>(m_fieldHandleAndKind & FIELD_KIND_MASK);
}

FieldSeqNode(CORINFO_FIELD_HANDLE fieldHnd, FieldSeqNode* next) : m_fieldHnd(fieldHnd), m_next(next)
CORINFO_FIELD_HANDLE GetFieldHandle() const
{
assert(!IsPseudoField() && (GetFieldHandleValue() != NO_FIELD_HANDLE));
return GetFieldHandleValue();
}

CORINFO_FIELD_HANDLE GetFieldHandleValue() const
{
return CORINFO_FIELD_HANDLE(m_fieldHandleAndKind & ~FIELD_KIND_MASK);
}

// returns true when this is the pseudo #FirstElem field sequence
bool IsFirstElemFieldSeq();
bool IsFirstElemFieldSeq() const;

// returns true when this is the pseudo #ConstantIndex field sequence
bool IsConstantIndexFieldSeq();
bool IsConstantIndexFieldSeq() const;

// returns true when this is the the pseudo #FirstElem field sequence or the pseudo #ConstantIndex field sequence
bool IsPseudoField() const;

CORINFO_FIELD_HANDLE GetFieldHandle() const
bool IsStaticField() const
{
assert(!IsPseudoField() && (m_fieldHnd != nullptr));
return m_fieldHnd;
return (GetKind() == FieldKind::SimpleStatic) || (GetKind() == FieldKind::SharedStatic);
}

bool IsSharedStaticField() const
{
return GetKind() == FieldKind::SharedStatic;
}

FieldSeqNode* GetNext() const
{
return m_next;
}

FieldSeqNode* GetTail()
Expand All @@ -262,16 +297,17 @@ struct FieldSeqNode
return tail;
}

// Make sure this provides methods that allow it to be used as a KeyFuncs type in SimplerHash.
// Make sure this provides methods that allow it to be used as a KeyFuncs type in JitHashTable.
// Note that there is a one-to-one relationship between the field handle and the field kind, so
// we do not need to mask away the latter for comparison purposes.
static int GetHashCode(FieldSeqNode fsn)
{
return static_cast<int>(reinterpret_cast<intptr_t>(fsn.m_fieldHnd)) ^
static_cast<int>(reinterpret_cast<intptr_t>(fsn.m_next));
return static_cast<int>(fsn.m_fieldHandleAndKind) ^ static_cast<int>(reinterpret_cast<intptr_t>(fsn.m_next));
}

static bool Equals(const FieldSeqNode& fsn1, const FieldSeqNode& fsn2)
{
return fsn1.m_fieldHnd == fsn2.m_fieldHnd && fsn1.m_next == fsn2.m_next;
return fsn1.m_fieldHandleAndKind == fsn2.m_fieldHandleAndKind && fsn1.m_next == fsn2.m_next;
}
};

Expand All @@ -293,7 +329,8 @@ class FieldSeqStore
FieldSeqStore(CompAllocator alloc);

// Returns the (canonical in the store) singleton field sequence for the given handle.
FieldSeqNode* CreateSingleton(CORINFO_FIELD_HANDLE fieldHnd);
FieldSeqNode* CreateSingleton(CORINFO_FIELD_HANDLE fieldHnd,
FieldSeqNode::FieldKind fieldKind = FieldSeqNode::FieldKind::Instance);

// This is a special distinguished FieldSeqNode indicating that a constant does *not*
// represent a valid field sequence. This is "infectious", in the sense that appending it
Expand Down Expand Up @@ -556,6 +593,7 @@ enum GenTreeFlags : unsigned int
GTF_ICON_CIDMID_HDL = 0x0E000000, // GT_CNS_INT -- constant is a class ID or a module ID
GTF_ICON_BBC_PTR = 0x0F000000, // GT_CNS_INT -- constant is a basic block count pointer
GTF_ICON_STATIC_BOX_PTR = 0x10000000, // GT_CNS_INT -- constant is an address of the box for a STATIC_IN_HEAP field
GTF_ICON_FIELD_SEQ = 0x11000000, // <--------> -- constant is a FieldSeqNode* (used only as VNHandle)

// GTF_ICON_REUSE_REG_VAL = 0x00800000 // GT_CNS_INT -- GTF_REUSE_REG_VAL, defined above
GTF_ICON_FIELD_OFF = 0x00400000, // GT_CNS_INT -- constant is a field offset
Expand Down
Loading