Skip to content

Commit

Permalink
Rework JIT<->EE communication for struct promotion
Browse files Browse the repository at this point in the history
Rework the communication so that the JIT asks the EE for struct field
related information relevant to promotion in a single JIT-EE call of a
new function called 'flattenType'.

As a side effect, removes CORINFO_FLG_CUSTOMLAYOUT and
CORINFO_FLG_INDEXABLE_FIELDS (the latter by adding support to inline
arrays in this new function). Also, last but not least, this adds
support for general recursive struct promotion provided the field limit
of 4 is not hit (for now, let's see if there are any more fundamental
issues with this).

Fix dotnet#85511
Fix dotnet#71711
  • Loading branch information
jakobbotsch committed Apr 30, 2023
1 parent b7d36a8 commit 5a69e5c
Show file tree
Hide file tree
Showing 23 changed files with 621 additions and 555 deletions.
44 changes: 42 additions & 2 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -857,10 +857,8 @@ enum CorInfoFlag
CORINFO_FLG_OVERLAPPING_FIELDS = 0x00100000, // struct or class has fields that overlap (aka union)
CORINFO_FLG_INTERFACE = 0x00200000, // it is an interface
CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't ask field info (used for types outside of AOT compilation version bubble)
CORINFO_FLG_CUSTOMLAYOUT = 0x00800000, // does this struct have custom layout?
CORINFO_FLG_CONTAINS_GC_PTR = 0x01000000, // does the class contain a gc ptr ?
CORINFO_FLG_DELEGATE = 0x02000000, // is this a subclass of delegate or multicast delegate ?
CORINFO_FLG_INDEXABLE_FIELDS = 0x04000000, // struct fields may be accessed via indexing (used for inline arrays)
CORINFO_FLG_BYREF_LIKE = 0x08000000, // it is byref-like value type
CORINFO_FLG_VARIANCE = 0x10000000, // MethodTable::HasVariance (sealed does *not* mean uncast-able)
CORINFO_FLG_BEFOREFIELDINIT = 0x20000000, // Additional flexibility for when to run .cctor (see code:#ClassConstructionFlags)
Expand Down Expand Up @@ -1946,6 +1944,25 @@ struct CORINFO_VarArgInfo
// (The CORINFO_VARARGS_HANDLE counts as an arg)
};

struct CORINFO_FLATTENED_TYPE_FIELD
{
// Primitive type, or CORINFO_TYPE_VALUECLASS for value classes marked as intrinsic.
CorInfoType type;
// For an intrinsic value class this is the handle for it.
CORINFO_CLASS_HANDLE intrinsicValueClassHnd;
// Offset of the field into the root struct.
unsigned offset;
// Field handle (only used for diagnostic purposes)
CORINFO_FIELD_HANDLE fieldHandle;
};

enum class FlattenTypeResult
{
Success,
Partial,
Failure,
};

#define SIZEOF__CORINFO_Object TARGET_POINTER_SIZE /* methTable */

#define CORINFO_Array_MaxLength 0x7FFFFFC7
Expand Down Expand Up @@ -2448,6 +2465,29 @@ class ICorStaticInfo
int32_t num
) = 0;

//------------------------------------------------------------------------------
// flattenType: Flatten a type into its primitive/intrinsic constituent
// fields.
//
// Parameters:
// clsHnd - Handle of the type.
// fields - [out] Pointer to entries to write.
// numFields - [in, out] Size of 'fields'. Updated to contain
// the number of entries written in 'fields'.
// significantPadding - [out] Whether data not covered by fields should
// be considered as significant or whether the JIT
// is allowed to discard it on copies.
//
// Returns:
// A result indicating whether the type was successfully flattened and
// whether the result is partial or not.
//
virtual FlattenTypeResult flattenType(
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_FLATTENED_TYPE_FIELD* fields,
size_t* numFields,
bool* significantPadding) = 0;

virtual bool checkMethodModifier(
CORINFO_METHOD_HANDLE hMethod,
const char * modifier,
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,12 @@ CORINFO_FIELD_HANDLE getFieldInClass(
CORINFO_CLASS_HANDLE clsHnd,
int32_t num) override;

FlattenTypeResult flattenType(
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_FLATTENED_TYPE_FIELD* fields,
size_t* numFields,
bool* significantPadding) override;

bool checkMethodModifier(
CORINFO_METHOD_HANDLE hMethod,
const char* modifier,
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* 387bcec3-9a71-4422-a11c-e7ce3b73c592 */
0x387bcec3,
0x9a71,
0x4422,
{0xa1, 0x1c, 0xe7, 0xce, 0x3b, 0x73, 0xc5, 0x92}
constexpr GUID JITEEVersionIdentifier = { /* 878d63a7-ffc9-421f-81f7-db4729f0ed5c */
0x878d63a7,
0xffc9,
0x421f,
{0x81, 0xf7, 0xdb, 0x47, 0x29, 0xf0, 0xed, 0x5c}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/ICorJitInfo_names_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ DEF_CLR_API(getClassAlignmentRequirement)
DEF_CLR_API(getClassGClayout)
DEF_CLR_API(getClassNumInstanceFields)
DEF_CLR_API(getFieldInClass)
DEF_CLR_API(flattenType)
DEF_CLR_API(checkMethodModifier)
DEF_CLR_API(getNewHelper)
DEF_CLR_API(getNewArrHelper)
Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,18 @@ CORINFO_FIELD_HANDLE WrapICorJitInfo::getFieldInClass(
return temp;
}

FlattenTypeResult WrapICorJitInfo::flattenType(
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_FLATTENED_TYPE_FIELD* fields,
size_t* numFields,
bool* significantPadding)
{
API_ENTER(flattenType);
FlattenTypeResult temp = wrapHnd->flattenType(clsHnd, fields, numFields, significantPadding);
API_LEAVE(flattenType);
return temp;
}

bool WrapICorJitInfo::checkMethodModifier(
CORINFO_METHOD_HANDLE hMethod,
const char* modifier,
Expand Down
31 changes: 10 additions & 21 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -592,9 +592,12 @@ class LclVarDsc
// 32-bit target. For implicit byref parameters, this gets hijacked between
// fgRetypeImplicitByRefArgs and fgMarkDemotedImplicitByRefArgs to indicate whether
// references to the arg are being rewritten as references to a promoted shadow local.
unsigned char lvIsStructField : 1; // Is this local var a field of a promoted struct local?
unsigned char lvContainsHoles : 1; // True when we have a promoted struct that contains holes
unsigned char lvCustomLayout : 1; // True when this struct has "CustomLayout"
unsigned char lvIsStructField : 1; // Is this local var a field of a promoted struct local?
unsigned char lvContainsHoles : 1; // Is this a promoted struct whose fields do not cover the struct local?
unsigned char lvAnySignificantPadding : 1; // True when we have a promoted struct that has significant padding in it
// Significant padding is any data in the struct that is not covered by a
// promoted field and that the EE told us we need to preserve on block copies
// inits.

unsigned char lvIsMultiRegArg : 1; // true if this is a multireg LclVar struct used in an argument context
unsigned char lvIsMultiRegRet : 1; // true if this is a multireg LclVar struct assigned from a multireg call
Expand Down Expand Up @@ -3539,8 +3542,8 @@ class Compiler
struct lvaStructFieldInfo
{
CORINFO_FIELD_HANDLE fldHnd;
unsigned char fldOffset;
unsigned char fldOrdinal;
uint8_t fldOffset;
uint8_t fldOrdinal;
var_types fldType;
unsigned fldSize;
CORINFO_CLASS_HANDLE fldTypeHnd;
Expand All @@ -3557,27 +3560,19 @@ class Compiler
CORINFO_CLASS_HANDLE typeHnd;
bool canPromote;
bool containsHoles;
bool customLayout;
bool fieldsSorted;
bool anySignificantPadding;
unsigned char fieldCnt;
lvaStructFieldInfo fields[MAX_NumOfFieldsInPromotableStruct];

lvaStructPromotionInfo(CORINFO_CLASS_HANDLE typeHnd = nullptr)
: typeHnd(typeHnd)
, canPromote(false)
, containsHoles(false)
, customLayout(false)
, fieldsSorted(false)
, anySignificantPadding(false)
, fieldCnt(0)
{
}
};

struct lvaFieldOffsetCmp
{
bool operator()(const lvaStructFieldInfo& field1, const lvaStructFieldInfo& field2);
};

// This class is responsible for checking validity and profitability of struct promotion.
// If it is both legal and profitable, then TryPromoteStructVar promotes the struct and initializes
// necessary information for fgMorphStructField to use.
Expand All @@ -3597,12 +3592,6 @@ class Compiler
bool CanPromoteStructVar(unsigned lclNum);
bool ShouldPromoteStructVar(unsigned lclNum);
void PromoteStructVar(unsigned lclNum);
void SortStructFields();

bool CanConstructAndPromoteField(lvaStructPromotionInfo* structPromotionInfo);

lvaStructFieldInfo GetFieldInfo(CORINFO_FIELD_HANDLE fieldHnd, BYTE ordinal);
bool TryPromoteStructField(lvaStructFieldInfo& outerFieldInfo);

private:
Compiler* compiler;
Expand Down
10 changes: 0 additions & 10 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4271,21 +4271,11 @@ inline static bool StructHasOverlappingFields(DWORD attribs)
return ((attribs & CORINFO_FLG_OVERLAPPING_FIELDS) != 0);
}

inline static bool StructHasCustomLayout(DWORD attribs)
{
return ((attribs & CORINFO_FLG_CUSTOMLAYOUT) != 0);
}

inline static bool StructHasDontDigFieldsFlagSet(DWORD attribs)
{
return ((attribs & CORINFO_FLG_DONT_DIG_FIELDS) != 0);
}

inline static bool StructHasIndexableFields(DWORD attribs)
{
return ((attribs & CORINFO_FLG_INDEXABLE_FIELDS) != 0);
}

//------------------------------------------------------------------------------
// DEBUG_DESTROY_NODE: sets value of tree to garbage to catch extra references
//
Expand Down
21 changes: 3 additions & 18 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11700,28 +11700,13 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack)
{
for (unsigned index = 0; index < varDsc->lvFieldCnt; index++)
{
unsigned fieldLclNum = varDsc->lvFieldLclStart + index;
LclVarDsc* fieldVarDsc = lvaGetDesc(fieldLclNum);
const char* fieldName;
#if !defined(TARGET_64BIT)
if (varTypeIsLong(varDsc))
{
fieldName = (index == 0) ? "lo" : "hi";
}
else
#endif // !defined(TARGET_64BIT)
{
CORINFO_CLASS_HANDLE typeHnd = varDsc->GetLayout()->GetClassHandle();
CORINFO_FIELD_HANDLE fldHnd =
info.compCompHnd->getFieldInClass(typeHnd, fieldVarDsc->lvFldOrdinal);
fieldName = eeGetFieldName(fldHnd, true, buffer, sizeof(buffer));
}
unsigned fieldLclNum = varDsc->lvFieldLclStart + index;
LclVarDsc* fieldVarDsc = lvaGetDesc(fieldLclNum);

printf("\n");
printf(" ");
printIndent(indentStack);
printf(" %-6s V%02u.%s (offs=0x%02x) -> ", varTypeName(fieldVarDsc->TypeGet()),
tree->AsLclVarCommon()->GetLclNum(), fieldName, fieldVarDsc->lvFldOffset);
printf(" %-6s %s -> ", varTypeName(fieldVarDsc->TypeGet()), fieldVarDsc->lvReason);
gtDispLclVar(fieldLclNum);
gtDispSsaName(fieldLclNum, tree->AsLclVarCommon()->GetSsaNum(this, index), isDef);

Expand Down
Loading

0 comments on commit 5a69e5c

Please sign in to comment.