Skip to content

Commit

Permalink
Revert "[crossgen2] Promote single byref aot. (#65682)" (#66946)
Browse files Browse the repository at this point in the history
This reverts commit c6ca9dc.
  • Loading branch information
jakobbotsch authored Mar 21, 2022
1 parent e22ebdf commit c992dc2
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 84 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ enum CorInfoFlag
CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently)
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, AOT can't rely on it (used for types outside of AOT compilation version bubble)
CORINFO_FLG_DONT_PROMOTE = 0x00400000, // don't try to promote fields (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 ?
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4172,8 +4172,6 @@ class Compiler
void PromoteStructVar(unsigned lclNum);
void SortStructFields();

bool CanConstructAndPromoteField(lvaStructPromotionInfo* structPromotionInfo);

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

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4485,9 +4485,9 @@ inline static bool StructHasCustomLayout(DWORD attribs)
return ((attribs & CORINFO_FLG_CUSTOMLAYOUT) != 0);
}

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

//------------------------------------------------------------------------------
Expand Down
100 changes: 23 additions & 77 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1707,6 +1707,13 @@ bool Compiler::StructPromotionHelper::CanPromoteStructType(CORINFO_CLASS_HANDLE
structPromotionInfo.fieldCnt = (unsigned char)fieldCnt;
DWORD typeFlags = compHandle->getClassAttribs(typeHnd);

if (StructHasNoPromotionFlagSet(typeFlags))
{
// In AOT ReadyToRun compilation, don't try to promote fields of types
// outside of the current version bubble.
return false;
}

bool overlappingFields = StructHasOverlappingFields(typeFlags);
if (overlappingFields)
{
Expand All @@ -1724,26 +1731,6 @@ bool Compiler::StructPromotionHelper::CanPromoteStructType(CORINFO_CLASS_HANDLE
unsigned structAlignment = roundUp(compHandle->getClassAlignmentRequirement(typeHnd), TARGET_POINTER_SIZE);
#endif // TARGET_ARM

// If we have "Custom Layout" then we might have an explicit Size attribute
// Managed C++ uses this for its structs, such C++ types will not contain GC pointers.
//
// The current VM implementation also incorrectly sets the CORINFO_FLG_CUSTOMLAYOUT
// whenever a managed value class contains any GC pointers.
// (See the comment for VMFLAG_NOT_TIGHTLY_PACKED in class.h)
//
// It is important to struct promote managed value classes that have GC pointers
// So we compute the correct value for "CustomLayout" here
//
if (StructHasCustomLayout(typeFlags) && ((typeFlags & CORINFO_FLG_CONTAINS_GC_PTR) == 0))
{
structPromotionInfo.customLayout = true;
}

if (StructHasDontDigFieldsFlagSet(typeFlags))
{
return CanConstructAndPromoteField(&structPromotionInfo);
}

unsigned fieldsSize = 0;

for (BYTE ordinal = 0; ordinal < fieldCnt; ++ordinal)
Expand Down Expand Up @@ -1795,6 +1782,21 @@ bool Compiler::StructPromotionHelper::CanPromoteStructType(CORINFO_CLASS_HANDLE
noway_assert((containsGCpointers == false) ||
((typeFlags & (CORINFO_FLG_CONTAINS_GC_PTR | CORINFO_FLG_BYREF_LIKE)) != 0));

// If we have "Custom Layout" then we might have an explicit Size attribute
// Managed C++ uses this for its structs, such C++ types will not contain GC pointers.
//
// The current VM implementation also incorrectly sets the CORINFO_FLG_CUSTOMLAYOUT
// whenever a managed value class contains any GC pointers.
// (See the comment for VMFLAG_NOT_TIGHTLY_PACKED in class.h)
//
// It is important to struct promote managed value classes that have GC pointers
// So we compute the correct value for "CustomLayout" here
//
if (StructHasCustomLayout(typeFlags) && ((typeFlags & CORINFO_FLG_CONTAINS_GC_PTR) == 0))
{
structPromotionInfo.customLayout = true;
}

// Check if this promoted struct contains any holes.
assert(!overlappingFields);
if (fieldsSize != structSize)
Expand All @@ -1810,62 +1812,6 @@ bool Compiler::StructPromotionHelper::CanPromoteStructType(CORINFO_CLASS_HANDLE
return true;
}

//--------------------------------------------------------------------------------------------
// CanConstructAndPromoteField - checks if we can construct field types without asking about them directly.
//
// Arguments:
// structPromotionInfo - struct promotion candidate information.
//
// Return value:
// true if we can figure out the fields from available knowledge.
//
// Notes:
// This is needed for AOT R2R compilation when we can't cross compilation bubble borders
// so we should not ask about fields that are not directly referenced. If we do VM will have
// to emit a type check for this field type but it does not have enough information about it.
// As a workaround for perfomance critical corner case: struct with 1 gcref, we try to construct
// the field information from indirect observations.
//
bool Compiler::StructPromotionHelper::CanConstructAndPromoteField(lvaStructPromotionInfo* structPromotionInfo)
{
const CORINFO_CLASS_HANDLE typeHnd = structPromotionInfo->typeHnd;
const COMP_HANDLE compHandle = compiler->info.compCompHnd;
const DWORD typeFlags = compHandle->getClassAttribs(typeHnd);
if (structPromotionInfo->fieldCnt != 1)
{
// Can't find out values for several fields.
return false;
}
if ((typeFlags & CORINFO_FLG_CONTAINS_GC_PTR) == 0)
{
// Can't find out type of a non-gc field.
return false;
}

const unsigned structSize = compHandle->getClassSize(typeHnd);
if (structSize != TARGET_POINTER_SIZE)
{
return false;
}

assert(!structPromotionInfo->containsHoles);
assert(!structPromotionInfo->customLayout);
lvaStructFieldInfo& fldInfo = structPromotionInfo->fields[0];

fldInfo.fldHnd = compHandle->getFieldInClass(typeHnd, 0);

// We should not read it anymore.
fldInfo.fldTypeHnd = 0;

fldInfo.fldOffset = 0;
fldInfo.fldOrdinal = 0;
fldInfo.fldSize = TARGET_POINTER_SIZE;
fldInfo.fldType = TYP_BYREF;

structPromotionInfo->canPromote = true;
return true;
}

//--------------------------------------------------------------------------------------------
// CanPromoteStructVar - checks if the struct can be promoted.
//
Expand Down Expand Up @@ -2892,7 +2838,7 @@ void Compiler::makeExtraStructQueries(CORINFO_CLASS_HANDLE structHandle, int lev
assert(structHandle != NO_CLASS_HANDLE);
(void)typGetObjLayout(structHandle);
DWORD typeFlags = info.compCompHnd->getClassAttribs(structHandle);
if (StructHasDontDigFieldsFlagSet(typeFlags))
if (StructHasNoPromotionFlagSet(typeFlags))
{
// In AOT ReadyToRun compilation, don't query fields of types
// outside of the current version bubble.
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2028,7 +2028,7 @@ private uint getClassAttribsInternal(TypeDesc type)
if (!_compilation.CompilationModuleGroup.VersionsWithType(type))
{
// Prevent the JIT from drilling into types outside of the current versioning bubble
result |= CorInfoFlag.CORINFO_FLG_DONT_DIG_FIELDS;
result |= CorInfoFlag.CORINFO_FLG_DONT_PROMOTE;
result &= ~CorInfoFlag.CORINFO_FLG_BEFOREFIELDINIT;
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ public enum CorInfoFlag : uint
CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently)
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 try to ask about fields outside of AOT compilation version bubble
CORINFO_FLG_DONT_PROMOTE = 0x00400000, // don't try to promote fieds of 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 ?
Expand Down

0 comments on commit c992dc2

Please sign in to comment.