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

Actually properly type primary selectors #63752

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 11 additions & 5 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6731,6 +6731,12 @@ class Compiler
CALLINT_ALL, // kills everything (normal method call)
};

enum class FieldKindForVN
{
SimpleStatic,
WithBaseAddr
};

public:
// A "LoopDsc" describes a ("natural") loop. We (currently) require the body of a loop to be a contiguous (in
// bbNext order) sequence of basic blocks. (At times, we may require the blocks in a loop to be "properly numbered"
Expand Down Expand Up @@ -6785,9 +6791,9 @@ class Compiler
int lpLoopVarFPCount; // The register count for the FP LclVars that are read/written inside this loop
int lpVarInOutFPCount; // The register count for the FP LclVars that are alive inside or across this loop

typedef JitHashTable<CORINFO_FIELD_HANDLE, JitPtrKeyFuncs<struct CORINFO_FIELD_STRUCT_>, bool> FieldHandleSet;
FieldHandleSet* lpFieldsModified; // This has entries (mappings to "true") for all static field and object
// instance fields modified
typedef JitHashTable<CORINFO_FIELD_HANDLE, JitPtrKeyFuncs<struct CORINFO_FIELD_STRUCT_>, FieldKindForVN>
FieldHandleSet;
FieldHandleSet* lpFieldsModified; // This has entries for all static field and object instance fields modified
// in the loop.

typedef JitHashTable<CORINFO_CLASS_HANDLE, JitPtrKeyFuncs<struct CORINFO_CLASS_STRUCT_>, bool> ClassHandleSet;
Expand All @@ -6798,7 +6804,7 @@ class Compiler
// Adds the variable liveness information for 'blk' to 'this' LoopDsc
void AddVariableLiveness(Compiler* comp, BasicBlock* blk);

inline void AddModifiedField(Compiler* comp, CORINFO_FIELD_HANDLE fldHnd);
inline void AddModifiedField(Compiler* comp, CORINFO_FIELD_HANDLE fldHnd, FieldKindForVN fieldKind);
// This doesn't *always* take a class handle -- it can also take primitive types, encoded as class handles
// (shifted left, with a low-order bit set to distinguish.)
// Use the {Encode/Decode}ElemType methods to construct/destruct these.
Expand Down Expand Up @@ -7045,7 +7051,7 @@ class Compiler
void AddVariableLivenessAllContainingLoops(unsigned lnum, BasicBlock* blk);

// Adds "fldHnd" to the set of modified fields of "lnum" and any parent loops.
void AddModifiedFieldAllContainingLoops(unsigned lnum, CORINFO_FIELD_HANDLE fldHnd);
void AddModifiedFieldAllContainingLoops(unsigned lnum, CORINFO_FIELD_HANDLE fldHnd, FieldKindForVN fieldKind);

// Adds "elemType" to the set of modified array element types of "lnum" and any parent loops.
void AddModifiedElemTypeAllContainingLoops(unsigned lnum, CORINFO_CLASS_HANDLE elemType);
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3346,14 +3346,14 @@ inline void Compiler::optAssertionRemove(AssertionIndex index)
}
}

inline void Compiler::LoopDsc::AddModifiedField(Compiler* comp, CORINFO_FIELD_HANDLE fldHnd)
inline void Compiler::LoopDsc::AddModifiedField(Compiler* comp, CORINFO_FIELD_HANDLE fldHnd, FieldKindForVN fieldKind)
{
if (lpFieldsModified == nullptr)
{
lpFieldsModified =
new (comp->getAllocatorLoopHoist()) Compiler::LoopDsc::FieldHandleSet(comp->getAllocatorLoopHoist());
}
lpFieldsModified->Set(fldHnd, true, FieldHandleSet::Overwrite);
lpFieldsModified->Set(fldHnd, fieldKind, FieldHandleSet::Overwrite);
}

inline void Compiler::LoopDsc::AddModifiedElemType(Compiler* comp, CORINFO_CLASS_HANDLE structHnd)
Expand Down
30 changes: 15 additions & 15 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15930,26 +15930,26 @@ bool GenTreeIntConCommon::AddrNeedsReloc(Compiler* comp)
// this: ADD(CONST FldSeq, baseAddr)
//
// Arguments:
// comp - the Compiler object
// pObj - [out] parameter for instance fields, the object reference
// pStatic - [out] parameter for static fields, the base address
// pFldSeq - [out] parameter for the field sequence
// comp - the Compiler object
// pBaseAddr - [out] parameter for "the base address"
// pFldSeq - [out] parameter for the field sequence
//
// Return Value:
// If "this" matches patterns denoted above, and the FldSeq found is "full",
// i. e. starts with a class field or a static field, and includes all the
// struct fields that this tree represents the address of, this method will
// return "true" and set either "pObj" or "pStatic" to some value, which must
// be used by the caller as the key into the "first field map" to obtain the
// actual value for the field.
// return "true" and set either "pBaseAddr" to some value, which must be used
// by the caller as the key into the "first field map" to obtain the actual
// value for the field. For instance fields, "base address" will be the object
// reference, for statics - the address to which the field offset with the
// field sequence is added, see "impImportStaticFieldAccess" and "fgMorphField".
//
bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pObj, GenTree** pStatic, FieldSeqNode** pFldSeq)
bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pFldSeq)
{
assert(TypeIs(TYP_I_IMPL, TYP_BYREF, TYP_REF));

*pObj = nullptr;
*pStatic = nullptr;
*pFldSeq = FieldSeqStore::NotAField();
*pBaseAddr = nullptr;
*pFldSeq = FieldSeqStore::NotAField();

GenTree* baseAddr = nullptr;
FieldSeqNode* fldSeq = nullptr;
Expand Down Expand Up @@ -16006,17 +16006,17 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pObj, GenTree** pStatic, Fie
// 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.
*pStatic = baseAddr;
*pFldSeq = fldSeq;
*pBaseAddr = baseAddr;
*pFldSeq = fldSeq;
return true;
}

if (baseAddr->TypeIs(TYP_REF))
{
assert(!comp->eeIsValueClass(comp->info.compCompHnd->getFieldClass(fldSeq->GetFieldHandle())));

*pObj = baseAddr;
*pFldSeq = fldSeq;
*pBaseAddr = baseAddr;
*pFldSeq = fldSeq;
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -1945,7 +1945,7 @@ struct GenTree
// where Y is an arbitrary tree, and X is a lclVar.
unsigned IsLclVarUpdateTree(GenTree** otherTree, genTreeOps* updateOper);

bool IsFieldAddr(Compiler* comp, GenTree** pObj, GenTree** pStatic, FieldSeqNode** pFldSeq);
bool IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pFldSeq);

// Requires "this" to be the address of an array (the child of a GT_IND labeled with GTF_IND_ARR_INDEX).
// Sets "pArr" to the node representing the array (either an array object pointer, or perhaps a byref to the some
Expand Down
31 changes: 12 additions & 19 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7850,24 +7850,16 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
}
else
{
// We are only interested in IsFieldAddr()'s fldSeq out parameter.
//
GenTree* obj = nullptr; // unused
GenTree* staticOffset = nullptr; // unused
FieldSeqNode* fldSeq = nullptr;

if (arg->IsFieldAddr(this, &obj, &staticOffset, &fldSeq) &&
(fldSeq != FieldSeqStore::NotAField()))
GenTree* baseAddr = nullptr;
FieldSeqNode* fldSeq = nullptr;
if (arg->IsFieldAddr(this, &baseAddr, &fldSeq))
{
// Get the first (object) field from field seq. GcHeap[field] will yield the "field map".
assert(fldSeq != nullptr);
if (fldSeq->IsFirstElemFieldSeq())
{
fldSeq = fldSeq->m_next;
assert(fldSeq != nullptr);
}
assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField()) &&
!fldSeq->IsPseudoField());

AddModifiedFieldAllContainingLoops(mostNestedLoop, fldSeq->m_fieldHnd);
FieldKindForVN fieldKind =
(baseAddr != nullptr) ? FieldKindForVN::WithBaseAddr : FieldKindForVN::SimpleStatic;
AddModifiedFieldAllContainingLoops(mostNestedLoop, fldSeq->GetFieldHandle(), fieldKind);
// Conservatively assume byrefs may alias this object.
memoryHavoc |= memoryKindSet(ByrefExposed);
}
Expand All @@ -7893,7 +7885,8 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
}
else if (lhs->OperGet() == GT_CLS_VAR)
{
AddModifiedFieldAllContainingLoops(mostNestedLoop, lhs->AsClsVar()->gtClsVarHnd);
AddModifiedFieldAllContainingLoops(mostNestedLoop, lhs->AsClsVar()->gtClsVarHnd,
FieldKindForVN::SimpleStatic);
// Conservatively assume byrefs may alias this static field
memoryHavoc |= memoryKindSet(ByrefExposed);
}
Expand Down Expand Up @@ -8073,12 +8066,12 @@ void Compiler::AddVariableLivenessAllContainingLoops(unsigned lnum, BasicBlock*
}

// Adds "fldHnd" to the set of modified fields of "lnum" and any parent loops.
void Compiler::AddModifiedFieldAllContainingLoops(unsigned lnum, CORINFO_FIELD_HANDLE fldHnd)
void Compiler::AddModifiedFieldAllContainingLoops(unsigned lnum, CORINFO_FIELD_HANDLE fldHnd, FieldKindForVN fieldKind)
{
assert(0 <= lnum && lnum < optLoopCount);
while (lnum != BasicBlock::NOT_IN_LOOP)
{
optLoopTable[lnum].AddModifiedField(this, fldHnd);
optLoopTable[lnum].AddModifiedField(this, fldHnd, fieldKind);
lnum = optLoopTable[lnum].lpParent;
}
}
Expand Down
Loading