Skip to content

Physical value numbering #68712

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

Merged
merged 17 commits into from
May 6, 2022
Merged

Physical value numbering #68712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
90cb15f
Work around FIELD turning into IND(struct)
SingleAccretion Apr 29, 2022
2f7141a
Delete "ROH"
SingleAccretion Apr 20, 2022
4d45ecc
Physical VN -- locals
SingleAccretion Apr 18, 2022
9a59a6f
Physical VN -- fields
SingleAccretion Apr 19, 2022
e8758c6
Physical VN -- arrays
SingleAccretion Apr 20, 2022
670a5cd
Deleting VNApplySelectors[Assign]
SingleAccretion Apr 20, 2022
b1950c9
Delete VNForEmptyMap
SingleAccretion Apr 25, 2022
10e1807
Add function headers
SingleAccretion Apr 20, 2022
f97c1ad
fgValueNumberLocalStore: fix arg ordering
SingleAccretion Apr 25, 2022
caa1998
Add BitCast<type <- ZeroObj> folding
SingleAccretion Apr 29, 2022
221fe64
Fix block assignment numbering
SingleAccretion Apr 29, 2022
fc03ae6
Add more TODO-PhysicalVN notes
SingleAccretion Apr 30, 2022
535b406
Update the documentation on maps and selection
SingleAccretion Apr 30, 2022
64b3cf6
Use a 'switch' in VNForMapSelectWork
SingleAccretion May 1, 2022
0c6af71
Fix formatting
SingleAccretion Apr 30, 2022
d1d5aea
Fix the CAST<float <-> integer> bug
SingleAccretion May 1, 2022
051224d
Correct documentation mistakes
SingleAccretion May 5, 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
53 changes: 19 additions & 34 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4692,40 +4692,25 @@ class Compiler
// tree node).
void fgValueNumber();

// Computes new GcHeap VN via the assignment H[elemTypeEq][arrVN][inx][fldSeq] = rhsVN.
// Assumes that "elemTypeEq" is the (equivalence class rep) of the array element type.
// The 'indType' is the indirection type of the lhs of the assignment and will typically
// match the element type of the array or fldSeq. When this type doesn't match
// or if the fldSeq is 'NotAField' we invalidate the array contents H[elemTypeEq][arrVN]
//
ValueNum fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq,
ValueNum arrVN,
ValueNum inxVN,
FieldSeqNode* fldSeq,
ValueNum rhsVN,
var_types indType);

// Requires that "tree" is a GT_IND marked as an array index, and that its address argument
// has been parsed to yield the other input arguments. If evaluation of the address
// can raise exceptions, those should be captured in the exception set "addrXvnp".
// Assumes that "elemTypeEq" is the (equivalence class rep) of the array element type.
// Marks "tree" with the VN for H[elemTypeEq][arrVN][inx][fldSeq] (for the liberal VN; a new unique
// VN for the conservative VN.) Also marks the tree's argument as the address of an array element.
// The type tree->TypeGet() will typically match the element type of the array or fldSeq.
// When this type doesn't match or if the fldSeq is 'NotAField' we return a new unique VN
//
ValueNum fgValueNumberArrIndexVal(GenTree* tree,
CORINFO_CLASS_HANDLE elemTypeEq,
ValueNum arrVN,
ValueNum inxVN,
ValueNumPair addrXvnp,
FieldSeqNode* fldSeq);

// Requires "funcApp" to be a VNF_PtrToArrElem, and "addrXvnp" to represent the exception set thrown
// by evaluating the array index expression "tree". Returns the value number resulting from
// dereferencing the array in the current GcHeap state. If "tree" is non-null, it must be the
// "GT_IND" that does the dereference, and it is given the returned value number.
ValueNum fgValueNumberArrIndexVal(GenTree* tree, VNFuncApp* funcApp, ValueNumPair addrXvnp);
void fgValueNumberLocalStore(GenTree* storeNode,
GenTreeLclVarCommon* lclDefNode,
ssize_t offset,
unsigned storeSize,
ValueNumPair value,
bool normalize = true);

void fgValueNumberArrayElemLoad(GenTree* loadTree, VNFuncApp* addrFunc);

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

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

void fgValueNumberFieldStore(GenTree* storeNode,
GenTree* baseAddr,
FieldSeqNode* 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
100 changes: 74 additions & 26 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15997,7 +15997,7 @@ bool GenTree::IsPartialLclFld(Compiler* comp)
(comp->lvaTable[this->AsLclVarCommon()->GetLclNum()].lvExactSize != genTypeSize(gtType)));
}

bool GenTree::DefinesLocal(Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bool* pIsEntire)
bool GenTree::DefinesLocal(Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bool* pIsEntire, ssize_t* pOffset)
{
GenTreeBlk* blkNode = nullptr;
if (OperIs(GT_ASG))
Expand All @@ -16017,12 +16017,17 @@ bool GenTree::DefinesLocal(Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bo
*pIsEntire = true;
}
}
if (pOffset != nullptr)
{
*pOffset = AsOp()->gtOp1->AsLclVarCommon()->GetLclOffs();
}
return true;
}
else if (AsOp()->gtOp1->OperGet() == GT_IND)
{
GenTree* indArg = AsOp()->gtOp1->AsOp()->gtOp1;
return indArg->DefinesLocalAddr(comp, genTypeSize(AsOp()->gtOp1->TypeGet()), pLclVarTree, pIsEntire);
return indArg->DefinesLocalAddr(comp, genTypeSize(AsOp()->gtOp1->TypeGet()), pLclVarTree, pIsEntire,
pOffset);
}
else if (AsOp()->gtOp1->OperIsBlk())
{
Expand All @@ -16038,7 +16043,7 @@ bool GenTree::DefinesLocal(Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bo
}

unsigned size = comp->typGetObjLayout(AsCall()->gtRetClsHnd)->GetSize();
return retBufArg->DefinesLocalAddr(comp, size, pLclVarTree, pIsEntire);
return retBufArg->DefinesLocalAddr(comp, size, pLclVarTree, pIsEntire, pOffset);
}
else if (OperIsBlk())
{
Expand All @@ -16064,14 +16069,14 @@ bool GenTree::DefinesLocal(Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bo
}
}

return destAddr->DefinesLocalAddr(comp, width, pLclVarTree, pIsEntire);
return destAddr->DefinesLocalAddr(comp, width, pLclVarTree, pIsEntire, pOffset);
}
// Otherwise...
return false;
}

// Returns true if this GenTree defines a result which is based on the address of a local.
bool GenTree::DefinesLocalAddr(Compiler* comp, unsigned width, GenTreeLclVarCommon** pLclVarTree, bool* pIsEntire)
bool GenTree::DefinesLocalAddr(
Compiler* comp, unsigned width, GenTreeLclVarCommon** pLclVarTree, bool* pIsEntire, ssize_t* pOffset)
{
if (OperIs(GT_ADDR, GT_LCL_VAR_ADDR, GT_LCL_FLD_ADDR))
{
Expand All @@ -16085,10 +16090,10 @@ bool GenTree::DefinesLocalAddr(Compiler* comp, unsigned width, GenTreeLclVarComm
{
GenTreeLclVarCommon* addrArgLcl = addrArg->AsLclVarCommon();
*pLclVarTree = addrArgLcl;
unsigned lclOffset = addrArgLcl->GetLclOffs();

if (pIsEntire != nullptr)
{
unsigned lclOffset = addrArgLcl->GetLclOffs();

if (lclOffset != 0)
{
// We aren't updating the bytes at [0..lclOffset-1] so *pIsEntire should be set to false
Expand All @@ -16107,29 +16112,45 @@ bool GenTree::DefinesLocalAddr(Compiler* comp, unsigned width, GenTreeLclVarComm
*pIsEntire = (varWidth == width);
}
}

if (pOffset != nullptr)
{
*pOffset += lclOffset;
}

return true;
}
else if (addrArg->OperGet() == GT_IND)
{
// A GT_ADDR of a GT_IND can both be optimized away, recurse using the child of the GT_IND
return addrArg->AsOp()->gtOp1->DefinesLocalAddr(comp, width, pLclVarTree, pIsEntire);
return addrArg->AsOp()->gtOp1->DefinesLocalAddr(comp, width, pLclVarTree, pIsEntire, pOffset);
}
}
else if (OperGet() == GT_ADD)
{
if (AsOp()->gtOp1->IsCnsIntOrI())
{
if (pOffset != nullptr)
{
*pOffset += AsOp()->gtOp1->AsIntCon()->IconValue();
}

// If we just adding a zero then we allow an IsEntire match against width
// otherwise we change width to zero to disallow an IsEntire Match
return AsOp()->gtOp2->DefinesLocalAddr(comp, AsOp()->gtOp1->IsIntegralConst(0) ? width : 0, pLclVarTree,
pIsEntire);
pIsEntire, pOffset);
}
else if (AsOp()->gtOp2->IsCnsIntOrI())
{
if (pOffset != nullptr)
{
*pOffset += AsOp()->gtOp2->AsIntCon()->IconValue();
}

// If we just adding a zero then we allow an IsEntire match against width
// otherwise we change width to zero to disallow an IsEntire Match
return AsOp()->gtOp1->DefinesLocalAddr(comp, AsOp()->gtOp2->IsIntegralConst(0) ? width : 0, pLclVarTree,
pIsEntire);
pIsEntire, pOffset);
}
}
// Post rationalization we could have GT_IND(GT_LEA(..)) trees.
Expand All @@ -16145,20 +16166,20 @@ bool GenTree::DefinesLocalAddr(Compiler* comp, unsigned width, GenTreeLclVarComm
GenTree* index = AsOp()->gtOp2;
if (index != nullptr)
{
assert(!index->DefinesLocalAddr(comp, width, pLclVarTree, pIsEntire));
assert(!index->DefinesLocalAddr(comp, width, pLclVarTree, pIsEntire, pOffset));
}
#endif // DEBUG

// base
GenTree* base = AsOp()->gtOp1;
GenTree* base = AsAddrMode()->Base();
if (base != nullptr)
{
// Lea could have an Indir as its base.
if (base->OperGet() == GT_IND)
if (pOffset != nullptr)
{
base = base->AsOp()->gtOp1->gtEffectiveVal(/*commas only*/ true);
*pOffset += AsAddrMode()->Offset();
}
return base->DefinesLocalAddr(comp, width, pLclVarTree, pIsEntire);

return base->DefinesLocalAddr(comp, width, pLclVarTree, pIsEntire, pOffset);
}
}
// Otherwise...
Expand Down Expand Up @@ -16604,6 +16625,12 @@ ssize_t GenTreeIndir::Offset()
}
}

unsigned GenTreeIndir::Size() const
{
assert(isIndir() || OperIsBlk());
return OperIsBlk() ? AsBlk()->Size() : genTypeSize(TypeGet());
}

//------------------------------------------------------------------------
// GenTreeIntConCommon::ImmedValNeedsReloc: does this immediate value needs recording a relocation with the VM?
//
Expand Down Expand Up @@ -16731,6 +16758,7 @@ bool GenTreeIntConCommon::AddrNeedsReloc(Compiler* comp)
// comp - the Compiler object
// pBaseAddr - [out] parameter for "the base address"
// pFldSeq - [out] parameter for the field sequence
// pOffset - [out] parameter for the offset of the component struct fields
//
// Return Value:
// If "this" matches patterns denoted above, and the FldSeq found is "full",
Expand All @@ -16742,7 +16770,7 @@ bool GenTreeIntConCommon::AddrNeedsReloc(Compiler* comp)
// 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** pBaseAddr, FieldSeqNode** pFldSeq)
bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pFldSeq, ssize_t* pOffset)
{
assert(TypeIs(TYP_I_IMPL, TYP_BYREF, TYP_REF));

Expand All @@ -16751,6 +16779,7 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF

GenTree* baseAddr = nullptr;
FieldSeqNode* fldSeq = FieldSeqStore::NotAField();
ssize_t offset = 0;

if (OperIs(GT_ADD))
{
Expand All @@ -16759,14 +16788,16 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF
if (AsOp()->gtOp1->IsCnsIntOrI() && AsOp()->gtOp1->IsIconHandle())
{
assert(!AsOp()->gtOp2->IsCnsIntOrI() || !AsOp()->gtOp2->IsIconHandle());
fldSeq = AsOp()->gtOp1->AsIntCon()->gtFieldSeq;
baseAddr = AsOp()->gtOp2;
fldSeq = AsOp()->gtOp1->AsIntCon()->gtFieldSeq;
offset = AsOp()->gtOp1->AsIntCon()->IconValue();
}
else if (AsOp()->gtOp2->IsCnsIntOrI())
{
assert(!AsOp()->gtOp1->IsCnsIntOrI() || !AsOp()->gtOp1->IsIconHandle());
fldSeq = AsOp()->gtOp2->AsIntCon()->gtFieldSeq;
baseAddr = AsOp()->gtOp1;
fldSeq = AsOp()->gtOp2->AsIntCon()->gtFieldSeq;
offset = AsOp()->gtOp2->AsIntCon()->IconValue();
}
else
{
Expand All @@ -16778,12 +16809,15 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF
else if (IsCnsIntOrI() && IsIconHandle(GTF_ICON_STATIC_HDL))
{
assert(!comp->GetZeroOffsetFieldMap()->Lookup(this) && (AsIntCon()->gtFieldSeq != nullptr));
fldSeq = AsIntCon()->gtFieldSeq;
baseAddr = this;
fldSeq = AsIntCon()->gtFieldSeq;
offset = AsIntCon()->IconValue();
}
else if (comp->GetZeroOffsetFieldMap()->Lookup(this, &fldSeq))
{
assert((fldSeq != FieldSeqStore::NotAField()) || (fldSeq->GetOffset() == 0));
baseAddr = this;
offset = 0;
}
else
{
Expand All @@ -16797,6 +16831,9 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF
return false;
}

// Subtract from the offset such that the portion remaining is relative to the field itself.
offset -= fldSeq->GetOffset();

// The above screens out obviously invalid cases, but we have more checks to perform. The
// sequence returned from this method *must* start with either a class (NOT struct) field
// or a static field. To avoid the expense of calling "getFieldClass" here, we will instead
Expand All @@ -16811,6 +16848,7 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF
}

*pFldSeq = fldSeq;
*pOffset = offset;
return true;
}

Expand All @@ -16820,6 +16858,7 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF

*pBaseAddr = baseAddr;
*pFldSeq = fldSeq;
*pOffset = offset;
return true;
}

Expand Down Expand Up @@ -18088,16 +18127,18 @@ bool GenTree::IsArrayAddr(GenTreeArrAddr** pArrAddr)
// 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::FieldKind::Instance);
FieldSeqNode FieldSeqStore::s_notAField(nullptr, nullptr, 0, 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::FieldKind fieldKind)
FieldSeqNode* FieldSeqStore::CreateSingleton(CORINFO_FIELD_HANDLE fieldHnd,
size_t offset,
FieldSeqNode::FieldKind fieldKind)
{
FieldSeqNode fsn(fieldHnd, nullptr, fieldKind);
FieldSeqNode fsn(fieldHnd, nullptr, offset, fieldKind);
FieldSeqNode* res = nullptr;
if (m_canonMap->Lookup(fsn, &res))
{
Expand Down Expand Up @@ -18136,7 +18177,7 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b)
assert(a != b);

FieldSeqNode* tmp = Append(a->GetNext(), b);
FieldSeqNode fsn(a->GetFieldHandleValue(), tmp, a->GetKind());
FieldSeqNode fsn(a->GetFieldHandleValue(), tmp, a->GetOffset(), a->GetKind());
FieldSeqNode* res = nullptr;
if (m_canonMap->Lookup(fsn, &res))
{
Expand All @@ -18152,7 +18193,8 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b)
}
}

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

Expand All @@ -18162,6 +18204,7 @@ FieldSeqNode::FieldSeqNode(CORINFO_FIELD_HANDLE fieldHnd, FieldSeqNode* next, Fi
if (fieldHnd != NO_FIELD_HANDLE)
{
assert(JitTls::GetCompiler()->eeIsFieldStatic(fieldHnd) == IsStaticField());
// TODO-PhysicalVN: assert that "offset" is correct.
}
else
{
Expand Down Expand Up @@ -22816,6 +22859,11 @@ unsigned GenTreeHWIntrinsic::GetResultOpNumForFMA(GenTree* use, GenTree* op1, Ge
}
#endif // TARGET_XARCH && FEATURE_HW_INTRINSICS

unsigned GenTreeLclFld::GetSize() const
{
return genTypeSize(TypeGet());
}

#ifdef TARGET_ARM
//------------------------------------------------------------------------
// IsOffsetMisaligned: check if the field needs a special handling on arm.
Expand Down
Loading