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

Revert "[crossgen2] Promote single byref aot. (#65682)" #66946

Merged
merged 1 commit into from
Mar 21, 2022
Merged

Revert "[crossgen2] Promote single byref aot. (#65682)" #66946

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: 1 addition & 1 deletion src/coreclr/inc/corinfo.h
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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