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

Jump to bottom

Proper VNs for zeroed structs #61285

Merged
merged 2 commits into from
Nov 9, 2021
Merged

Proper VNs for zeroed structs #61285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
132f31f
Number zero-initialized struct properly
SingleAccretion Nov 5, 2021
c884372
Fix VNZeroForType's handling of SIMD types
SingleAccretion Nov 5, 2021
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: 1 addition & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7986,7 +7986,7 @@ class Compiler
bool eeIsJitIntrinsic(CORINFO_METHOD_HANDLE ftn);
bool eeIsFieldStatic(CORINFO_FIELD_HANDLE fldHnd);

var_types eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd);
var_types eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd = nullptr);

#if defined(DEBUG) || defined(FEATURE_JIT_METHOD_PERF) || defined(FEATURE_SIMD) || defined(TRACK_LSRA_STATS)

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/ee_il_dll.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ bool Compiler::eeIsFieldStatic(CORINFO_FIELD_HANDLE fldHnd)
}

FORCEINLINE
var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd)
var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd)
{
return JITtype2varType(info.compCompHnd->getFieldType(fldHnd));
return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd));
}

FORCEINLINE
Expand Down
133 changes: 82 additions & 51 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,11 +464,9 @@ ValueNumStore::ValueNumStore(Compiler* comp, CompAllocator alloc)
{
m_VNsForSmallIntConsts[i] = NoVN;
}
// We will reserve chunk 0 to hold some special constants, like the constant NULL, the "exception" value, and the
// "zero map."
// We will reserve chunk 0 to hold some special constants.
Chunk* specialConstChunk = new (m_alloc) Chunk(m_alloc, &m_nextChunkBase, TYP_REF, CEA_Const);
specialConstChunk->m_numUsed +=
SRC_NumSpecialRefConsts; // Implicitly allocate 0 ==> NULL, and 1 ==> Exception, 2 ==> ZeroMap.
specialConstChunk->m_numUsed += SRC_NumSpecialRefConsts;
ChunkNum cn = m_chunks.Push(specialConstChunk);
assert(cn == 0);

Expand Down Expand Up @@ -1787,8 +1785,6 @@ ValueNum ValueNumStore::VNForHandle(ssize_t cnsVal, GenTreeFlags handleFlags)
}
}

// Returns the value number for zero of the given "typ".
// It has an unreached() for a "typ" that has no zero value, such as TYP_VOID.
ValueNum ValueNumStore::VNZeroForType(var_types typ)
{
switch (typ)
Expand All @@ -1812,15 +1808,18 @@ ValueNum ValueNumStore::VNZeroForType(var_types typ)
return VNForNull();
case TYP_BYREF:
return VNForByrefCon(0);
case TYP_STRUCT:
return VNForZeroMap(); // Recursion!

#ifdef FEATURE_SIMD
case TYP_SIMD8:
case TYP_SIMD12:
case TYP_SIMD16:
case TYP_SIMD32:
return VNForLongCon(0);
// We do not have the base type - a "fake" one will have to do. Note that we cannot
// use the HWIntrinsic "get_Zero" VNFunc here. This is because they only represent
// "fully zeroed" vectors, and here we may be loading one from memory, leaving upper
// bits undefined. So using "SIMD_Init" is "the next best thing", so to speak, and
// TYP_FLOAT is one of the more popular base types, so that's why we use it here.
return VNForFunc(typ, VNF_SIMD_Init, VNForFloatCon(0), VNForSimdType(genTypeSize(typ), TYP_FLOAT));
#endif // FEATURE_SIMD

// These should be unreached.
Expand All @@ -1829,6 +1828,16 @@ ValueNum ValueNumStore::VNZeroForType(var_types typ)
}
}

ValueNum ValueNumStore::VNForZeroObj(CORINFO_CLASS_HANDLE structHnd)
{
assert(structHnd != NO_CLASS_HANDLE);

ValueNum structHndVN = VNForHandle(ssize_t(structHnd), GTF_ICON_CLASS_HDL);
ValueNum zeroObjVN = VNForFunc(TYP_STRUCT, VNF_ZeroObj, structHndVN);

return zeroObjVN;
}

// Returns the value number for one of the given "typ".
// It returns NoVN for a "typ" that has no one value, such as TYP_REF.
ValueNum ValueNumStore::VNOneForType(var_types typ)
Expand Down Expand Up @@ -1856,6 +1865,17 @@ ValueNum ValueNumStore::VNOneForType(var_types typ)
}
}

#ifdef FEATURE_SIMD
ValueNum ValueNumStore::VNForSimdType(unsigned simdSize, var_types simdBaseType)
{
ValueNum baseTypeVN = VNForIntCon(INT32(simdBaseType));
ValueNum sizeVN = VNForIntCon(simdSize);
ValueNum simdTypeVN = VNForFunc(TYP_REF, VNF_SimdType, sizeVN, baseTypeVN);

return simdTypeVN;
}
#endif // FEATURE_SIMD

class Object* ValueNumStore::s_specialRefConsts[] = {nullptr, nullptr, nullptr};

//----------------------------------------------------------------------------------------
Expand Down Expand Up @@ -2317,14 +2337,9 @@ ValueNum ValueNumStore::VNForMapSelectWork(
return RecursiveVN;
}

if (map == VNForZeroMap())
{
return VNZeroForType(type);
}
else if (IsVNFunc(map))
VNFuncApp funcApp;
if (GetVNFunc(map, &funcApp))
{
VNFuncApp funcApp;
GetVNFunc(map, &funcApp);
if (funcApp.m_func == VNF_MapStore)
{
// select(store(m, i, v), i) == v
Expand Down Expand Up @@ -2476,6 +2491,22 @@ ValueNum ValueNumStore::VNForMapSelectWork(
m_fixedPointMapSels.Pop();
}
}
else if (funcApp.m_func == VNF_ZeroObj)
{
// For structs, we need to extract the handle from the selector.
if (type == TYP_STRUCT)
{
// We only expect field selectors here.
assert(GetHandleFlags(index) == GTF_ICON_FIELD_HDL);
CORINFO_FIELD_HANDLE fieldHnd = CORINFO_FIELD_HANDLE(ConstantValue<ssize_t>(index));
CORINFO_CLASS_HANDLE structHnd = NO_CLASS_HANDLE;
m_pComp->eeGetFieldType(fieldHnd, &structHnd);

return VNForZeroObj(structHnd);
}

return VNZeroForType(type);
}
}

// We may have run out of budget and already assigned a result
Expand Down Expand Up @@ -5792,11 +5823,6 @@ void ValueNumStore::vnDump(Compiler* comp, ValueNum vn, bool isPtr)
{
printf("void");
}
else
{
assert(vn == VNForZeroMap());
printf("zeroMap");
}
break;
case TYP_BYREF:
printf("byrefVal");
Expand Down Expand Up @@ -5868,6 +5894,9 @@ void ValueNumStore::vnDump(Compiler* comp, ValueNum vn, bool isPtr)
case VNF_CastOvf:
vnDumpCast(comp, vn);
break;
case VNF_ZeroObj:
vnDumpZeroObj(comp, &funcApp);
break;

default:
printf("%s(", VNFuncName(funcApp.m_func));
Expand Down Expand Up @@ -6089,6 +6118,12 @@ void ValueNumStore::vnDumpCast(Compiler* comp, ValueNum castVN)
}
}

void ValueNumStore::vnDumpZeroObj(Compiler* comp, VNFuncApp* zeroObj)
{
printf("ZeroObj(");
comp->vnPrint(zeroObj->m_args[0], 0);
printf(": %s)", comp->eeGetClassName(CORINFO_CLASS_HANDLE(ConstantValue<ssize_t>(zeroObj->m_args[0]))));
}
#endif // DEBUG

// Static fields, methods.
Expand Down Expand Up @@ -6252,10 +6287,9 @@ static const char* s_reservedNameArr[] = {
"$VN.Recursive", // -2 RecursiveVN
"$VN.No", // -1 NoVN
"$VN.Null", // 0 VNForNull()
"$VN.ZeroMap", // 1 VNForZeroMap()
"$VN.ReadOnlyHeap", // 2 VNForROH()
"$VN.Void", // 3 VNForVoid()
"$VN.EmptyExcSet" // 4 VNForEmptyExcSet()
"$VN.ReadOnlyHeap", // 1 VNForROH()
"$VN.Void", // 2 VNForVoid()
"$VN.EmptyExcSet" // 3 VNForEmptyExcSet()
};

// Returns the string name of "vn" when it is a reserved value number, nullptr otherwise
Expand Down Expand Up @@ -6680,7 +6714,8 @@ void Compiler::fgValueNumber()
if (isZeroed)
{
// By default we will zero init these LclVars
initVal = vnStore->VNZeroForType(typ);
initVal = (typ == TYP_STRUCT) ? vnStore->VNForZeroObj(varDsc->GetStructHnd())
: vnStore->VNZeroForType(typ);
}
else
{
Expand Down Expand Up @@ -7942,39 +7977,38 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
// Should not have been recorded as updating the GC heap.
assert(!GetMemorySsaMap(GcHeap)->Lookup(tree));

unsigned lclNum = lclVarTree->GetLclNum();

unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree);

// Ignore vars that we excluded from SSA (for example, because they're address-exposed). They don't have
// SSA names in which to store VN's on defs. We'll yield unique VN's when we read from them.
if (lvaInSsa(lclNum) && lclDefSsaNum != SsaConfig::RESERVED_SSA_NUM)
if (lclDefSsaNum != SsaConfig::RESERVED_SSA_NUM)
{
LclVarDsc* lclVarDsc = lvaGetDesc(lclVarTree);

// Should not have been recorded as updating ByrefExposed.
assert(!GetMemorySsaMap(ByrefExposed)->Lookup(tree));

ValueNum initBlkVN = ValueNumStore::NoVN;
GenTree* initConst = rhs;
if (isEntire && initConst->OperGet() == GT_CNS_INT)
ValueNum lclVarVN = ValueNumStore::NoVN;
if (isEntire && rhs->IsIntegralConst(0))
{
unsigned initVal = 0xFF & (unsigned)initConst->AsIntConCommon()->IconValue();
if (initVal == 0)
{
initBlkVN = vnStore->VNZeroForType(lclVarTree->TypeGet());
}
// Note that it is possible to see pretty much any kind of type for the local
// (not just TYP_STRUCT) here because of the ASG(BLK(ADDR(LCL_VAR/FLD)), 0) form.
lclVarVN = (lclVarDsc->TypeGet() == TYP_STRUCT) ? vnStore->VNForZeroObj(lclVarDsc->GetStructHnd())
: vnStore->VNZeroForType(lclVarDsc->TypeGet());
}
else
{
// Non-zero block init is very rare so we'll use a simple, unique VN here.
lclVarVN = vnStore->VNForExpr(compCurBB, lclVarDsc->TypeGet());
}
ValueNum lclVarVN = (initBlkVN != ValueNumStore::NoVN)
? initBlkVN
: vnStore->VNForExpr(compCurBB, var_types(lvaTable[lclNum].lvType));

lvaTable[lclNum].GetPerSsaData(lclDefSsaNum)->m_vnPair.SetBoth(lclVarVN);
lclVarDsc->GetPerSsaData(lclDefSsaNum)->m_vnPair.SetBoth(lclVarVN);
#ifdef DEBUG
if (verbose)
{
printf("N%03u ", tree->gtSeqNum);
printf("Tree ");
Compiler::printTreeID(tree);
printf(" ");
gtDispNodeName(tree);
printf(" V%02u/%d => ", lclNum, lclDefSsaNum);
printf(" assigned VN to local var V%02u/%d: ", lclVarTree->GetLclNum(), lclDefSsaNum);
vnPrint(lclVarVN, 1);
printf("\n");
}
Expand Down Expand Up @@ -9492,9 +9526,7 @@ void Compiler::fgValueNumberSimd(GenTree* tree)

if (encodeResultType)
{
ValueNum vnSize = vnStore->VNForIntCon(simdNode->GetSimdSize());
ValueNum vnBaseType = vnStore->VNForIntCon(INT32(simdNode->GetSimdBaseType()));
ValueNum simdTypeVN = vnStore->VNForFunc(TYP_REF, VNF_SimdType, vnSize, vnBaseType);
ValueNum simdTypeVN = vnStore->VNForSimdType(simdNode->GetSimdSize(), simdNode->GetSimdBaseType());
resvnp.SetBoth(simdTypeVN);

#ifdef DEBUG
Expand Down Expand Up @@ -9609,9 +9641,8 @@ void Compiler::fgValueNumberHWIntrinsic(GenTree* tree)

if (encodeResultType)
{
ValueNum vnSize = vnStore->VNForIntCon(hwIntrinsicNode->GetSimdSize());
ValueNum vnBaseType = vnStore->VNForIntCon(INT32(hwIntrinsicNode->GetSimdBaseType()));
ValueNum simdTypeVN = vnStore->VNForFunc(TYP_REF, VNF_SimdType, vnSize, vnBaseType);
ValueNum simdTypeVN =
vnStore->VNForSimdType(hwIntrinsicNode->GetSimdSize(), hwIntrinsicNode->GetSimdBaseType());
resvnp.SetBoth(simdTypeVN);

#ifdef DEBUG
Expand Down
19 changes: 11 additions & 8 deletions src/coreclr/jit/valuenum.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,6 @@ class ValueNumStore
return ValueNum(SRC_Null);
}

// The zero map is the map that returns a zero "for the appropriate type" when indexed at any index.
static ValueNum VNForZeroMap()
{
// We reserve Chunk 0 for "special" VNs. Let SRC_ZeroMap (== 1) be the zero map.
return ValueNum(SRC_ZeroMap);
}

// The ROH map is the map for the "read-only heap". We assume that this is never mutated, and always
// has the same value number.
static ValueNum VNForROH()
Expand Down Expand Up @@ -463,10 +456,18 @@ class ValueNumStore
// It has an unreached() for a "typ" that has no zero value, such as TYP_VOID.
ValueNum VNZeroForType(var_types typ);

// Returns the value number for a zero-initialized struct.
ValueNum VNForZeroObj(CORINFO_CLASS_HANDLE structHnd);

// Returns the value number for one of the given "typ".
// It returns NoVN for a "typ" that has no one value, such as TYP_REF.
ValueNum VNOneForType(var_types typ);

#ifdef FEATURE_SIMD
// A helper function for constructing VNF_SimdType VNs.
ValueNum VNForSimdType(unsigned simdSize, var_types simdBaseType);
#endif // FEATURE_SIMD

// Create or return the existimg value number representing a singleton exception set
// for the the exception value "x".
ValueNum VNExcSetSingleton(ValueNum x);
Expand Down Expand Up @@ -1021,6 +1022,9 @@ class ValueNumStore
// Prints the cast's representation mirroring GT_CAST's dump format.
void vnDumpCast(Compiler* comp, ValueNum castVN);

// Requires "zeroObj" to be a VNF_ZeroObj. Prints its representation.
void vnDumpZeroObj(Compiler* comp, VNFuncApp* zeroObj);

// Returns the string name of "vnf".
static const char* VNFuncName(VNFunc vnf);
// Used in the implementation of the above.
Expand Down Expand Up @@ -1449,7 +1453,6 @@ class ValueNumStore
enum SpecialRefConsts
{
SRC_Null,
SRC_ZeroMap,
SRC_ReadOnlyHeap,
SRC_Void,
SRC_EmptyExcSet,
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/valuenumfuncs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ ValueNumFuncDef(MapSelect, 2, false, false, false) // Args: 0: map, 1: key.

ValueNumFuncDef(FieldSeq, 2, false, false, false) // Sequence (VN of null == empty) of (VN's of) field handles.
ValueNumFuncDef(NotAField, 0, false, false, false) // Value number function for FieldSeqStore::NotAField.
ValueNumFuncDef(ZeroMap, 0, false, false, false) // The "ZeroMap": indexing at any index yields "zero of the desired type".

ValueNumFuncDef(PtrToLoc, 2, false, false, false) // Pointer (byref) to a local variable. Args: VN's of: 0: var num, 1: FieldSeq.
ValueNumFuncDef(PtrToArrElem, 4, false, false, false) // Pointer (byref) to an array element. Args: 0: array elem type eq class var_types value, VN's of: 1: array, 2: index, 3: FieldSeq.
Expand All @@ -22,6 +21,7 @@ ValueNumFuncDef(Phi, 2, false, false, false) // A phi function. Only occ
ValueNumFuncDef(PhiDef, 3, false, false, false) // Args: 0: local var # (or -1 for memory), 1: SSA #, 2: VN of definition.
// Wouldn't need this if I'd made memory a regular local variable...
ValueNumFuncDef(PhiMemoryDef, 2, false, false, false) // Args: 0: VN for basic block pointer, 1: VN of definition
ValueNumFuncDef(ZeroObj, 1, false, false, false) // Zero-initialized struct. Args: 0: VN of the class handle.
ValueNumFuncDef(InitVal, 1, false, false, false) // An input arg, or init val of a local Args: 0: a constant VN.


Expand Down