Skip to content

Type memory maps with placeholders #61933

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 1 commit into from
Dec 1, 2021
Merged

Type memory maps with placeholders #61933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5477,6 +5477,8 @@ class Compiler
// For a store to an address-exposed local at curTree, record the new curMemoryVN and update curTree's MemorySsaMap.
void recordAddressExposedLocalStore(GenTree* curTree, ValueNum memoryVN DEBUGARG(const char* msg));

void fgSetCurrentMemoryVN(MemoryKind memoryKind, ValueNum newMemoryVN);

// Tree caused an update in the current memory VN. If "tree" has an associated heap SSA #, record that
// value in that SSA #.
void fgValueNumberRecordMemorySsa(MemoryKind memoryKind, GenTree* tree);
Expand Down
93 changes: 56 additions & 37 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2217,7 +2217,7 @@ ValueNum ValueNumStore::VNForMapStore(ValueNum map, ValueNum index, ValueNum val
if (m_pComp->verbose)
{
printf(" VNForMapStore(" FMT_VN ", " FMT_VN ", " FMT_VN "):%s in " FMT_BB " returns ", map, index, value,
varTypeName(TypeOfVN(result)), bb->bbNum);
VNMapTypeName(TypeOfVN(result)), bb->bbNum);
m_pComp->vnPrint(result, 1);
printf("\n");
}
Expand Down Expand Up @@ -2255,7 +2255,7 @@ ValueNum ValueNumStore::VNForMapSelect(ValueNumKind vnk, var_types type, ValueNu
#ifdef DEBUG
if (m_pComp->verbose)
{
printf(" VNForMapSelect(" FMT_VN ", " FMT_VN "):%s returns ", map, index, varTypeName(type));
printf(" VNForMapSelect(" FMT_VN ", " FMT_VN "):%s returns ", map, index, VNMapTypeName(type));
m_pComp->vnPrint(result, 1);
printf("\n");
}
Expand Down Expand Up @@ -4265,8 +4265,8 @@ ValueNum Compiler::fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq,
bool invalidateArray = false;
ValueNum elemTypeEqVN = vnStore->VNForHandle(ssize_t(elemTypeEq), GTF_ICON_CLASS_HDL);
var_types arrElemType = DecodeElemType(elemTypeEq);
ValueNum hAtArrType = vnStore->VNForMapSelect(VNK_Liberal, TYP_REF, fgCurMemoryVN[GcHeap], elemTypeEqVN);
ValueNum hAtArrTypeAtArr = vnStore->VNForMapSelect(VNK_Liberal, TYP_REF, hAtArrType, arrVN);
ValueNum hAtArrType = vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, fgCurMemoryVN[GcHeap], elemTypeEqVN);
ValueNum hAtArrTypeAtArr = vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, hAtArrType, arrVN);
ValueNum hAtArrTypeAtArrAtInx = vnStore->VNForMapSelect(VNK_Liberal, arrElemType, hAtArrTypeAtArr, inxVN);

ValueNum newValAtInx = ValueNumStore::NoVN;
Expand All @@ -4279,7 +4279,7 @@ ValueNum Compiler::fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq,
JITDUMP(" *** NotAField sequence encountered in fgValueNumberArrIndexAssign\n");

// Store a new unique value for newValAtArrType
newValAtArrType = vnStore->VNForExpr(compCurBB, TYP_REF);
newValAtArrType = vnStore->VNForExpr(compCurBB, TYP_MEM);
invalidateArray = true;
}
else
Expand All @@ -4301,7 +4301,7 @@ ValueNum Compiler::fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq,
JITDUMP(" *** Mismatched types in fgValueNumberArrIndexAssign\n");

// Store a new unique value for newValAtArrType
newValAtArrType = vnStore->VNForExpr(compCurBB, TYP_REF);
newValAtArrType = vnStore->VNForExpr(compCurBB, TYP_MEM);
invalidateArray = true;
}
}
Expand All @@ -4328,7 +4328,7 @@ ValueNum Compiler::fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq,
printf(" hAtArrTypeAtArr " FMT_VN " is MapSelect(hAtArrType(" FMT_VN "), arr=" FMT_VN ")\n", hAtArrTypeAtArr,
hAtArrType, arrVN);
printf(" hAtArrTypeAtArrAtInx " FMT_VN " is MapSelect(hAtArrTypeAtArr(" FMT_VN "), inx=" FMT_VN "):%s\n",
hAtArrTypeAtArrAtInx, hAtArrTypeAtArr, inxVN, varTypeName(arrElemType));
hAtArrTypeAtArrAtInx, hAtArrTypeAtArr, inxVN, ValueNumStore::VNMapTypeName(arrElemType));

if (!invalidateArray)
{
Expand Down Expand Up @@ -4401,8 +4401,8 @@ ValueNum Compiler::fgValueNumberArrIndexVal(GenTree* tree,
else
{
ValueNum elemTypeEqVN = vnStore->VNForHandle(ssize_t(elemTypeEq), GTF_ICON_CLASS_HDL);
ValueNum hAtArrType = vnStore->VNForMapSelect(VNK_Liberal, TYP_REF, fgCurMemoryVN[GcHeap], elemTypeEqVN);
ValueNum hAtArrTypeAtArr = vnStore->VNForMapSelect(VNK_Liberal, TYP_REF, hAtArrType, arrVN);
ValueNum hAtArrType = vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, fgCurMemoryVN[GcHeap], elemTypeEqVN);
ValueNum hAtArrTypeAtArr = vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, hAtArrType, arrVN);
ValueNum wholeElem = vnStore->VNForMapSelect(VNK_Liberal, elemTyp, hAtArrTypeAtArr, inxVN);

#ifdef DEBUG
Expand Down Expand Up @@ -6291,8 +6291,7 @@ const char* ValueNumStore::VNFuncNameArr[] = {
#undef ValueNumFuncDef
};

// static
const char* ValueNumStore::VNFuncName(VNFunc vnf)
/* static */ const char* ValueNumStore::VNFuncName(VNFunc vnf)
{
if (vnf < VNF_Boundary)
{
Expand All @@ -6304,6 +6303,19 @@ const char* ValueNumStore::VNFuncName(VNFunc vnf)
}
}

/* static */ const char* ValueNumStore::VNMapTypeName(var_types type)
{
switch (type)
{
case TYP_HEAP:
return "heap";
case TYP_MEM:
return "mem";
default:
return varTypeName(type);
}
}

static const char* s_reservedNameArr[] = {
"$VN.Recursive", // -2 RecursiveVN
"$VN.No", // -1 NoVN
Expand Down Expand Up @@ -6757,7 +6769,7 @@ void Compiler::fgValueNumber()
}
}
// Give memory an initial value number (about which we know nothing).
ValueNum memoryInitVal = vnStore->VNForFunc(TYP_REF, VNF_InitVal, vnStore->VNForIntCon(-1)); // Use -1 for memory.
ValueNum memoryInitVal = vnStore->VNForFunc(TYP_HEAP, VNF_InitVal, vnStore->VNForIntCon(-1)); // Use -1 for memory.
GetMemoryPerSsaData(SsaConfig::FIRST_SSA_NUM)->m_vnPair.SetBoth(memoryInitVal);
#ifdef DEBUG
if (verbose)
Expand Down Expand Up @@ -6894,8 +6906,8 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
// Is there a phi for this block?
if (blk->bbMemorySsaPhiFunc[memoryKind] == nullptr)
{
fgCurMemoryVN[memoryKind] = GetMemoryPerSsaData(blk->bbMemorySsaNumIn[memoryKind])->m_vnPair.GetLiberal();
assert(fgCurMemoryVN[memoryKind] != ValueNumStore::NoVN);
ValueNum newMemoryVN = GetMemoryPerSsaData(blk->bbMemorySsaNumIn[memoryKind])->m_vnPair.GetLiberal();
fgSetCurrentMemoryVN(memoryKind, newMemoryVN);
}
else
{
Expand Down Expand Up @@ -6943,7 +6955,7 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
unsigned phiArgSSANum = phiArgs->GetSsaNum();
ValueNum phiArgSSANumVN = vnStore->VNForIntCon(phiArgSSANum);
JITDUMP(" Building phi application: $%x = SSA# %d.\n", phiArgSSANumVN, phiArgSSANum);
phiAppVN = vnStore->VNForFunc(TYP_REF, VNF_Phi, phiArgSSANumVN, phiAppVN);
phiAppVN = vnStore->VNForFunc(TYP_HEAP, VNF_Phi, phiArgSSANumVN, phiAppVN);
JITDUMP(" Building phi application: $%x = phi($%x, $%x).\n", phiAppVN, phiArgSSANumVN,
oldPhiAppVN);
phiArgs = phiArgs->m_nextArg;
Expand All @@ -6954,16 +6966,16 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
}
else
{
newMemoryVN = vnStore->VNForFunc(TYP_REF, VNF_PhiMemoryDef,
newMemoryVN = vnStore->VNForFunc(TYP_HEAP, VNF_PhiMemoryDef,
vnStore->VNForHandle(ssize_t(blk), GTF_EMPTY), phiAppVN);
}
}
GetMemoryPerSsaData(blk->bbMemorySsaNumIn[memoryKind])->m_vnPair.SetLiberal(newMemoryVN);
fgCurMemoryVN[memoryKind] = newMemoryVN;
fgSetCurrentMemoryVN(memoryKind, newMemoryVN);
if ((memoryKind == GcHeap) && byrefStatesMatchGcHeapStates)
{
// Keep the CurMemoryVNs in sync
fgCurMemoryVN[ByrefExposed] = newMemoryVN;
fgSetCurrentMemoryVN(ByrefExposed, newMemoryVN);
}
}
#ifdef DEBUG
Expand Down Expand Up @@ -7061,7 +7073,7 @@ ValueNum Compiler::fgMemoryVNForLoopSideEffects(MemoryKind memoryKind,
// If this loop has memory havoc effects, just use a new, unique VN.
if (optLoopTable[loopNum].lpLoopHasMemoryHavoc[memoryKind])
{
ValueNum res = vnStore->VNForExpr(entryBlock, TYP_REF);
ValueNum res = vnStore->VNForExpr(entryBlock, TYP_HEAP);
#ifdef DEBUG
if (verbose)
{
Expand Down Expand Up @@ -7101,7 +7113,7 @@ ValueNum Compiler::fgMemoryVNForLoopSideEffects(MemoryKind memoryKind,
}
if (multipleNonLoopPreds)
{
ValueNum res = vnStore->VNForExpr(entryBlock, TYP_REF);
ValueNum res = vnStore->VNForExpr(entryBlock, TYP_HEAP);
#ifdef DEBUG
if (verbose)
{
Expand Down Expand Up @@ -7146,9 +7158,9 @@ ValueNum Compiler::fgMemoryVNForLoopSideEffects(MemoryKind memoryKind,
}
#endif // DEBUG

// Instance field maps get the placeholder TYP_REF - they do not represent "singular"
// Instance field maps get a placeholder type - they do not represent "singular"
// values. Static field maps, on the other hand, do, and so must be given proper types.
var_types fldMapType = eeIsFieldStatic(fldHnd) ? eeGetFieldType(fldHnd) : TYP_REF;
var_types fldMapType = eeIsFieldStatic(fldHnd) ? eeGetFieldType(fldHnd) : TYP_MEM;

newMemoryVN = vnStore->VNForMapStore(newMemoryVN, fldHndVN, vnStore->VNForExpr(entryBlock, fldMapType));
}
Expand Down Expand Up @@ -7181,7 +7193,7 @@ ValueNum Compiler::fgMemoryVNForLoopSideEffects(MemoryKind memoryKind,
#endif // DEBUG

ValueNum elemTypeVN = vnStore->VNForHandle(ssize_t(elemClsHnd), GTF_ICON_CLASS_HDL);
ValueNum uniqueVN = vnStore->VNForExpr(entryBlock, TYP_REF);
ValueNum uniqueVN = vnStore->VNForExpr(entryBlock, TYP_MEM);
newMemoryVN = vnStore->VNForMapStore(newMemoryVN, elemTypeVN, uniqueVN);
}
}
Expand Down Expand Up @@ -7209,35 +7221,35 @@ ValueNum Compiler::fgMemoryVNForLoopSideEffects(MemoryKind memoryKind,
void Compiler::fgMutateGcHeap(GenTree* tree DEBUGARG(const char* msg))
{
// Update the current memory VN, and if we're tracking the heap SSA # caused by this node, record it.
recordGcHeapStore(tree, vnStore->VNForExpr(compCurBB, TYP_REF) DEBUGARG(msg));
recordGcHeapStore(tree, vnStore->VNForExpr(compCurBB, TYP_HEAP) DEBUGARG(msg));
}

void Compiler::fgMutateAddressExposedLocal(GenTree* tree DEBUGARG(const char* msg))
{
// Update the current ByrefExposed VN, and if we're tracking the heap SSA # caused by this node, record it.
recordAddressExposedLocalStore(tree, vnStore->VNForExpr(compCurBB) DEBUGARG(msg));
recordAddressExposedLocalStore(tree, vnStore->VNForExpr(compCurBB, TYP_HEAP) DEBUGARG(msg));
}

void Compiler::recordGcHeapStore(GenTree* curTree, ValueNum gcHeapVN DEBUGARG(const char* msg))
{
// bbMemoryDef must include GcHeap for any block that mutates the GC Heap
// and GC Heap mutations are also ByrefExposed mutations
assert((compCurBB->bbMemoryDef & memoryKindSet(GcHeap, ByrefExposed)) == memoryKindSet(GcHeap, ByrefExposed));
fgCurMemoryVN[GcHeap] = gcHeapVN;
fgSetCurrentMemoryVN(GcHeap, gcHeapVN);

if (byrefStatesMatchGcHeapStates)
{
// Since GcHeap and ByrefExposed share SSA nodes, they need to share
// value numbers too.
fgCurMemoryVN[ByrefExposed] = gcHeapVN;
fgSetCurrentMemoryVN(ByrefExposed, gcHeapVN);
}
else
{
// GcHeap and ByrefExposed have different defnums and VNs. We conservatively
// assume that this GcHeap store may alias any byref load/store, so don't
// bother trying to record the map/select stuff, and instead just an opaque VN
// for ByrefExposed
fgCurMemoryVN[ByrefExposed] = vnStore->VNForExpr(compCurBB);
fgSetCurrentMemoryVN(ByrefExposed, vnStore->VNForExpr(compCurBB, TYP_HEAP));
}

#ifdef DEBUG
Expand All @@ -7262,7 +7274,7 @@ void Compiler::recordAddressExposedLocalStore(GenTree* curTree, ValueNum memoryV

// bbMemoryDef must include ByrefExposed for any block that mutates an address-exposed local
assert((compCurBB->bbMemoryDef & memoryKindSet(ByrefExposed)) != 0);
fgCurMemoryVN[ByrefExposed] = memoryVN;
fgSetCurrentMemoryVN(ByrefExposed, memoryVN);

#ifdef DEBUG
if (verbose)
Expand All @@ -7276,6 +7288,13 @@ void Compiler::recordAddressExposedLocalStore(GenTree* curTree, ValueNum memoryV
fgValueNumberRecordMemorySsa(ByrefExposed, curTree);
}

void Compiler::fgSetCurrentMemoryVN(MemoryKind memoryKind, ValueNum newMemoryVN)
{
assert(vnStore->VNIsValid(newMemoryVN));
assert(vnStore->TypeOfVN(newMemoryVN) == TYP_HEAP);
fgCurMemoryVN[memoryKind] = newMemoryVN;
}

void Compiler::fgValueNumberRecordMemorySsa(MemoryKind memoryKind, GenTree* tree)
{
unsigned ssaNum;
Expand Down Expand Up @@ -7516,7 +7535,7 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree)
// through address-taken locals in regions of code with no calls or byref
// writes.
// For now, just use a new opaque VN.
ValueNum heapVN = vnStore->VNForExpr(compCurBB);
ValueNum heapVN = vnStore->VNForExpr(compCurBB, TYP_HEAP);
recordAddressExposedLocalStore(tree, heapVN DEBUGARG("local assign"));
}
#ifdef DEBUG
Expand Down Expand Up @@ -7599,7 +7618,7 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree)
// As with GT_LCL_VAR, we could probably use MapStore here and MapSelect at corresponding
// loads, but to do so would have to identify the subset of address-exposed locals
// whose fields can be disambiguated.
ValueNum heapVN = vnStore->VNForExpr(compCurBB);
ValueNum heapVN = vnStore->VNForExpr(compCurBB, TYP_HEAP);
recordAddressExposedLocalStore(tree, heapVN DEBUGARG("local field assign"));
}
}
Expand Down Expand Up @@ -7725,7 +7744,7 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree)
// through address-taken locals in regions of code with no calls or byref
// writes.
// For now, just use a new opaque VN.
ValueNum heapVN = vnStore->VNForExpr(compCurBB);
ValueNum heapVN = vnStore->VNForExpr(compCurBB, TYP_HEAP);
recordAddressExposedLocalStore(tree, heapVN DEBUGARG("PtrToLoc indir"));
}
}
Expand Down Expand Up @@ -7841,8 +7860,8 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree)

// Construct the "field map" VN. It represents memory state of the first field
// of all objects on the heap. This is our primary map.
ValueNum fldMapVN = vnStore->VNForMapSelect(VNK_Liberal, firstFieldType,
fgCurMemoryVN[GcHeap], firstFieldSelectorVN);
ValueNum fldMapVN = vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, fgCurMemoryVN[GcHeap],
firstFieldSelectorVN);

ValueNum firstFieldValueSelectorVN = ValueNumStore::NoVN;
if (obj != nullptr)
Expand Down Expand Up @@ -7911,7 +7930,7 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree)
// through address-taken locals in regions of code with no calls or byref
// writes.
// For now, just use a new opaque VN.
ValueNum memoryVN = vnStore->VNForExpr(compCurBB);
ValueNum memoryVN = vnStore->VNForExpr(compCurBB, TYP_HEAP);
recordAddressExposedLocalStore(tree, memoryVN DEBUGARG("PtrToLoc indir"));
}
else if (!isLocal)
Expand Down Expand Up @@ -9028,8 +9047,8 @@ void Compiler::fgValueNumberTree(GenTree* tree)
ValueNum firstFieldSelectorVN =
vnStore->VNForFieldSelector(fldSeq2->GetFieldHandle(), &firstFieldType, &structSize);

ValueNum fldMapVN = vnStore->VNForMapSelect(VNK_Liberal, firstFieldType, fgCurMemoryVN[GcHeap],
firstFieldSelectorVN);
ValueNum fldMapVN =
vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, fgCurMemoryVN[GcHeap], firstFieldSelectorVN);

ValueNum firstFieldValueSelectorVN;
if (obj != nullptr)
Expand Down
24 changes: 12 additions & 12 deletions src/coreclr/jit/valuenum.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,9 @@
// taken from another map at a given index. As such, it must have a type that corresponds
// to the "index/selector", in practical terms - the field's type.
//
// Note that the above constraints on the types for maps are not followed currently to the
// letter by the implementation - "opaque" maps can and do have TYP_REF assigned to them
// in various situations such as when initializing the "primary" VNs for loop entries.
//
// Note as well that the meaning of "the type" for a map is overloaded, because maps are used
// both to represent memory "of all fields B of all objects that have this field in the heap"
// and "the field B of this particular object on the heap". Only the latter maps can be used
// as VNs for actual nodes, while the former are used for "the first field" maps and "array
// equivalence type" maps, and, of course, for the heap VNs, which always have the placeholder
// types of TYP_REF or TYP_UNKNOWN. In principle, placeholder types could be given to all the
// maps of the former type.
// Note that we give "placeholder" types (TYP_UNDEF and TYP_UNKNOWN as TYP_MEM and TYP_HEAP)
// to maps that do not represent values found in IR. This is just to avoid confusion and
// facilitate more precise validating checks.
//
// Let's review the following snippet to demonstrate how the MapSelect/MapStore machinery works
// together to deliver the results that it does. Say we have this snippet of (C#) code:
Expand Down Expand Up @@ -196,6 +188,12 @@ struct VNFuncApp
// This define is used with string concatenation to put this in printf format strings
#define FMT_VN "$%x"

// We will use this placeholder type for memory maps that do not represent IR values ("field maps", etc).
static const var_types TYP_MEM = TYP_UNDEF;

// We will use this placeholder type for memory maps representing "the heap" (GcHeap/ByrefExposed).
static const var_types TYP_HEAP = TYP_UNKNOWN;

class ValueNumStore
{

Expand Down Expand Up @@ -1037,9 +1035,11 @@ class ValueNumStore
// Used in the implementation of the above.
static const char* VNFuncNameArr[];

// Returns a type name used for "maps", i. e. displays TYP_UNDEF and TYP_UNKNOWN as TYP_MEM and TYP_HEAP.
static const char* VNMapTypeName(var_types type);

// Returns the string name of "vn" when it is a reserved value number, nullptr otherwise
static const char* reservedName(ValueNum vn);

#endif // DEBUG

// Returns true if "vn" is a reserved value number
Expand Down