Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions src/coreclr/jit/ssabuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1423,6 +1423,7 @@ void SsaBuilder::RenameVariables()
assert(varDsc->lvTracked);

if (varDsc->lvIsParam || m_pCompiler->info.compInitMem || varDsc->lvMustInit ||
!m_pCompiler->fgVarNeedsExplicitZeroInit(lclNum, false, false) ||
VarSetOps::IsMember(m_pCompiler, m_pCompiler->fgFirstBB->bbLiveIn, varDsc->lvVarIndex))
{
unsigned ssaNum = varDsc->lvPerSsaData.AllocSsaNum(m_allocator);
Expand Down
13 changes: 10 additions & 3 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2372,7 +2372,10 @@ private FlattenTypeResult FlattenTypeHelper(MetadataType type, uint baseOffs, CO
{
if (type.IsExplicitLayout || (type.IsSequentialLayout && type.GetClassLayout().Size != 0) || type.IsInlineArray)
{
significantPadding = true;
if (!type.ContainsGCPointers && !type.IsByRefLike)
{
significantPadding = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this pessimize cases that don't have padding?

Say for example you've defined a union { float f; int32_t i; } and therefore it is explicit, but the two structures are "perfectly overlapping".

-- JW as a potential "future" improvement, not something to be handled in this PR or anything

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, since we don't support promotion of structs with overlapping fields. But if we did still no as the JIT only pays attention to significantPadding when there are holes in the struct.

}
}

foreach (FieldDesc fd in type.GetFields())
Expand Down Expand Up @@ -2444,11 +2447,15 @@ private FlattenTypeResult FlattenTypeHelper(MetadataType type, uint baseOffs, CO

private FlattenTypeResult flattenType(CORINFO_CLASS_STRUCT_* clsHnd, CORINFO_FLATTENED_TYPE_FIELD* fields, UIntPtr* numFields, ref bool significantPadding)
{
MetadataType type = (MetadataType)HandleToObject(clsHnd);
TypeDesc type = HandleToObject(clsHnd);

if (type is not MetadataType metadataType || !type.IsValueType)
return FlattenTypeResult.Failure;

significantPadding = false;
nuint maxFields = *numFields;
*numFields = 0;
FlattenTypeResult result = FlattenTypeHelper(type, 0, fields, maxFields, numFields, ref significantPadding);
FlattenTypeResult result = FlattenTypeHelper(metadataType, 0, fields, maxFields, numFields, ref significantPadding);

#if READYTORUN
// TODO: Do we need a version bubble check here if we're going to
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/tools/superpmi/superpmi-shared/agnostic.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,22 @@ struct Agnostic_GetStaticFieldCurrentClass
bool isSpeculative;
};

struct Agnostic_CORINFO_FLATTENED_TYPE_FIELD
{
DWORDLONG intrinsicValueClassHnd;
DWORDLONG fieldHandle;
DWORD type;
DWORD offset;
};

struct Agnostic_FlattenTypeResult
{
DWORD result;
DWORD fieldsBuffer;
DWORD numFields;
DWORD significantPadding;
};

struct Agnostic_CORINFO_RESOLVED_TOKEN
{
Agnostic_CORINFO_RESOLVED_TOKENin inValue;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ LWM(GetObjectContent, DLDD, DD)
LWM(GetStaticFieldCurrentClass, DLD, Agnostic_GetStaticFieldCurrentClass)
LWM(GetFieldClass, DWORDLONG, DWORDLONG)
LWM(GetFieldInClass, DLD, DWORDLONG)
LWM(FlattenType, DLD, Agnostic_FlattenTypeResult)
LWM(GetFieldInfo, Agnostic_GetFieldInfo, Agnostic_CORINFO_FIELD_INFO)
LWM(GetFieldOffset, DWORDLONG, DWORD)
LWM(GetFieldThreadLocalStoreID, DWORDLONG, DLD)
Expand Down
66 changes: 66 additions & 0 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4848,6 +4848,72 @@ CORINFO_FIELD_HANDLE MethodContext::repGetFieldInClass(CORINFO_CLASS_HANDLE clsH
return result;
}

void MethodContext::recFlattenType(FlattenTypeResult result, CORINFO_CLASS_HANDLE clsHnd, CORINFO_FLATTENED_TYPE_FIELD* fields, size_t maxFields, size_t* numFields, bool* significantPadding)
{
if (FlattenType == nullptr)
FlattenType = new LightWeightMap<DLD, Agnostic_FlattenTypeResult>();

DLD key;
ZeroMemory(&key, sizeof(key));
key.A = CastHandle(clsHnd);
key.B = (DWORD)maxFields;

Agnostic_FlattenTypeResult value;
ZeroMemory(&value, sizeof(value));

value.result = (DWORD)result;
if (result == FlattenTypeResult::Failure)
{
value.fieldsBuffer = UINT_MAX;
}
else
{
Agnostic_CORINFO_FLATTENED_TYPE_FIELD* agnosticFields = new Agnostic_CORINFO_FLATTENED_TYPE_FIELD[*numFields];
for (size_t i = 0; i < *numFields; i++)
{
agnosticFields[i] = SpmiRecordsHelper::StoreAgnostic_CORINFO_FLATTENED_TYPE_FIELD(fields[i]);
}

value.fieldsBuffer = FlattenType->AddBuffer((unsigned char*)agnosticFields, (unsigned int)(sizeof(Agnostic_CORINFO_FLATTENED_TYPE_FIELD) * *numFields));
value.numFields = (DWORD)*numFields;
value.significantPadding = *significantPadding ? 1 : 0;

delete[] agnosticFields;
}

FlattenType->Add(key, value);
}
void MethodContext::dmpFlattenType(DLD key, const Agnostic_FlattenTypeResult& value)
{
printf("FlattenType key cls-%016" PRIX64 " fields-%d, value result=%d fields=%d", key.A, key.B, (DWORD)value.result, value.numFields);
}
FlattenTypeResult MethodContext::repFlattenType(CORINFO_CLASS_HANDLE clsHnd, CORINFO_FLATTENED_TYPE_FIELD* fields, size_t* numFields, bool* significantPadding)
{
DLD key;
ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding
key.A = CastHandle(clsHnd);
key.B = (DWORD)*numFields;

Agnostic_FlattenTypeResult value = LookupByKeyOrMiss(FlattenType, key, ": key cls-%016" PRIX64 " fields-%d", key.A, key.B);

FlattenTypeResult result = (FlattenTypeResult)value.result;
if (result == FlattenTypeResult::Failure)
{
return result;
}

Assert(value.numFields <= *numFields); // since it's part of the key
Agnostic_CORINFO_FLATTENED_TYPE_FIELD* valueFields = (Agnostic_CORINFO_FLATTENED_TYPE_FIELD*)FlattenType->GetBuffer(value.fieldsBuffer);
for (size_t i = 0; i < value.numFields; i++)
{
fields[i] = SpmiRecordsHelper::RestoreCORINFO_FLATTENED_TYPE_FIELD(valueFields[i]);
}

*numFields = value.numFields;
*significantPadding = value.significantPadding != 0;
return result;
}

void MethodContext::recGetFieldType(CORINFO_FIELD_HANDLE field,
CORINFO_CLASS_HANDLE* structType,
CORINFO_CLASS_HANDLE memberParent,
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,10 @@ class MethodContext
void dmpGetFieldInClass(DLD key, DWORDLONG value);
CORINFO_FIELD_HANDLE repGetFieldInClass(CORINFO_CLASS_HANDLE clsHnd, INT num);

void recFlattenType(FlattenTypeResult result, CORINFO_CLASS_HANDLE clsHnd, CORINFO_FLATTENED_TYPE_FIELD* fields, size_t maxFields, size_t* numFields, bool* significantPadding);
void dmpFlattenType(DLD key, const Agnostic_FlattenTypeResult& value);
FlattenTypeResult repFlattenType(CORINFO_CLASS_HANDLE clsHnd, CORINFO_FLATTENED_TYPE_FIELD* fields, size_t* numFields, bool* significantPadding);

void recGetFieldType(CORINFO_FIELD_HANDLE field,
CORINFO_CLASS_HANDLE* structType,
CORINFO_CLASS_HANDLE memberParent,
Expand Down Expand Up @@ -1192,6 +1196,7 @@ enum mcPackets
Packet_GetThreadLocalStaticBlocksInfo = 208,
Packet_GetRISCV64PassStructInRegisterFlags = 209,
Packet_GetObjectContent = 210,
Packet_FlattenType = 211,
};

void SetDebugDumpVariables();
Expand Down
23 changes: 23 additions & 0 deletions src/coreclr/tools/superpmi/superpmi-shared/spmirecordhelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ class SpmiRecordsHelper
static Agnostic_CORINFO_LOOKUP StoreAgnostic_CORINFO_LOOKUP(CORINFO_LOOKUP* pLookup);

static CORINFO_LOOKUP RestoreCORINFO_LOOKUP(Agnostic_CORINFO_LOOKUP& agnosticLookup);

static Agnostic_CORINFO_FLATTENED_TYPE_FIELD StoreAgnostic_CORINFO_FLATTENED_TYPE_FIELD(const CORINFO_FLATTENED_TYPE_FIELD& field);
static CORINFO_FLATTENED_TYPE_FIELD RestoreCORINFO_FLATTENED_TYPE_FIELD(const Agnostic_CORINFO_FLATTENED_TYPE_FIELD& field);
};

inline Agnostic_CORINFO_RESOLVED_TOKENin SpmiRecordsHelper::CreateAgnostic_CORINFO_RESOLVED_TOKENin(
Expand Down Expand Up @@ -542,4 +545,24 @@ inline CORINFO_LOOKUP SpmiRecordsHelper::RestoreCORINFO_LOOKUP(Agnostic_CORINFO_
return lookup;
}

inline Agnostic_CORINFO_FLATTENED_TYPE_FIELD SpmiRecordsHelper::StoreAgnostic_CORINFO_FLATTENED_TYPE_FIELD(const CORINFO_FLATTENED_TYPE_FIELD& field)
{
Agnostic_CORINFO_FLATTENED_TYPE_FIELD result;
result.type = (DWORD)field.type;
result.intrinsicValueClassHnd = CastHandle(field.intrinsicValueClassHnd);
result.offset = field.offset;
result.fieldHandle = CastHandle(field.fieldHandle);
return result;
}

inline CORINFO_FLATTENED_TYPE_FIELD SpmiRecordsHelper::RestoreCORINFO_FLATTENED_TYPE_FIELD(const Agnostic_CORINFO_FLATTENED_TYPE_FIELD& field)
{
CORINFO_FLATTENED_TYPE_FIELD result;
result.type = (CorInfoType)field.type;
result.intrinsicValueClassHnd = (CORINFO_CLASS_HANDLE)field.intrinsicValueClassHnd;
result.offset = field.offset;
result.fieldHandle = (CORINFO_FIELD_HANDLE)field.fieldHandle;
return result;
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,9 @@ FlattenTypeResult interceptor_ICJI::flattenType(
bool* significantPadding)
{
mc->cr->AddCall("flattenType");
size_t maxFields = *numFields;
FlattenTypeResult result = original_ICorJitInfo->flattenType(clsHnd, fields, numFields, significantPadding);
mc->recFlattenType(result, clsHnd, fields, maxFields, numFields, significantPadding);
return result;
}

Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -583,8 +583,7 @@ FlattenTypeResult MyICJI::flattenType(
bool* significantPadding)
{
jitInstance->mc->cr->AddCall("flattenType");
return FlattenTypeResult::Failure;
//return jitInstance->mc->repFlattenType(;
return jitInstance->mc->repFlattenType(clsHnd, fields, numFields, significantPadding);
}

bool MyICJI::checkMethodModifier(CORINFO_METHOD_HANDLE hMethod, LPCSTR modifier, bool fOptional)
Expand Down
30 changes: 23 additions & 7 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2229,7 +2229,14 @@ static FlattenTypeResult FlattenTypeHelper(
EEClass* pClass = pMT->GetClass();
if (pClass->IsNotTightlyPacked() && (!pClass->IsManagedSequential() || pClass->HasExplicitSize() || pClass->IsInlineArray()))
{
*significantPadding = true;
// Historically on the JIT side we did not consider types with GC
// pointers to have significant padding, even when they have explicit
// layout attributes. This retains the more liberal treatment and
// lets the JIT still optimize these cases.
if (!pMT->ContainsPointers() && pMT != g_TypedReferenceMT)
{
*significantPadding = true;
}
}

ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS);
Expand Down Expand Up @@ -2319,14 +2326,23 @@ FlattenTypeResult CEEInfo::flattenType(

JIT_TO_EE_TRANSITION_LEAF();

TypeHandle VMClsHnd(clsHnd);
TypeHandle typeHnd(clsHnd);

MethodTable* pMT = VMClsHnd.AsMethodTable();
if (typeHnd.IsNativeValueType())
{
*numFields = 0;
*significantPadding = true;
result = FlattenTypeResult::Success;
}
else if (typeHnd.IsValueType())
{
MethodTable* pMT = typeHnd.AsMethodTable();

size_t maxFields = *numFields;
*numFields = 0;
*significantPadding = false;
result = FlattenTypeHelper(pMT, 0, fields, maxFields, numFields, significantPadding);
size_t maxFields = *numFields;
*numFields = 0;
*significantPadding = false;
result = FlattenTypeHelper(pMT, 0, fields, maxFields, numFields, significantPadding);
}

EE_TO_JIT_TRANSITION_LEAF();

Expand Down