Skip to content

Commit

Permalink
Fix for Crossgen2 compilation of System.ServiceModel.Syndication (dot…
Browse files Browse the repository at this point in the history
…net#35728)

I have added a new flag CORINFO_FLG_DONT_PROMOTE
to suppress field promotion for types outside of the compilation
version bubble. I have also fixed two other small inconsistencies
I noticed in the investigation:

1) I have changed getClassName to use TypeString as the output
matches Crossgen1 and makes NGenDump outputs easier to compare.

2) When I was comparing the NGen dumps, I noticed a check in
Crossgen1 missing in Crossgen2 - only setting
CORINFO_FLG_BEFOREFIELDINIT for types in the same version bubble;
so I made it conditional in Crossgen2 too.

Thanks

Tomas
  • Loading branch information
trylek authored Jun 4, 2020
1 parent 4c23c57 commit 8d02f6a
Showing 5 changed files with 27 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/src/inc/corinfo.h
Original file line number Diff line number Diff line change
@@ -822,7 +822,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
// unused = 0x00400000,
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 ?
5 changes: 5 additions & 0 deletions src/coreclr/src/jit/compiler.hpp
Original file line number Diff line number Diff line change
@@ -4670,6 +4670,11 @@ inline static bool StructHasCustomLayout(DWORD attribs)
return ((attribs & CORINFO_FLG_CUSTOMLAYOUT) != 0);
}

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

/*****************************************************************************
* This node should not be referenced by anyone now. Set its values to garbage
* to catch extra references
7 changes: 7 additions & 0 deletions src/coreclr/src/jit/lclvars.cpp
Original file line number Diff line number Diff line change
@@ -1686,6 +1686,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)
{
15 changes: 13 additions & 2 deletions src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
@@ -1070,7 +1070,7 @@ private void resolveToken(ref CORINFO_RESOLVED_TOKEN pResolvedToken)

#if READYTORUN
TypeDesc owningType = methodIL.OwningMethod.GetTypicalMethodDefinition().OwningType;
bool recordToken = _compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(owningType) && owningType is EcmaType;
bool recordToken = _compilation.CompilationModuleGroup.VersionsWithType(owningType) && owningType is EcmaType;
#endif

if (result is MethodDesc)
@@ -1220,7 +1220,9 @@ private CorInfoType asCorInfoType(CORINFO_CLASS_STRUCT_* cls)
private byte* getClassName(CORINFO_CLASS_STRUCT_* cls)
{
var type = HandleToObject(cls);
return (byte*)GetPin(StringToUTF8(type.ToString()));
StringBuilder nameBuilder = new StringBuilder();
TypeString.Instance.AppendName(nameBuilder, type);
return (byte*)GetPin(StringToUTF8(nameBuilder.ToString()));
}

private byte* getClassNameFromMetadata(CORINFO_CLASS_STRUCT_* cls, byte** namespaceName)
@@ -1357,6 +1359,15 @@ private uint getClassAttribsInternal(TypeDesc type)
result |= CorInfoFlag.CORINFO_FLG_ABSTRACT;
}

#if READYTORUN
if (!_compilation.CompilationModuleGroup.VersionsWithType(type))
{
// Prevent the JIT from drilling into types outside of the current versioning bubble
result |= CorInfoFlag.CORINFO_FLG_DONT_PROMOTE;
result &= ~CorInfoFlag.CORINFO_FLG_BEFOREFIELDINIT;
}
#endif

return (uint)result;
}

2 changes: 1 addition & 1 deletion src/coreclr/src/tools/Common/JitInterface/CorInfoTypes.cs
Original file line number Diff line number Diff line change
@@ -634,7 +634,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_UNUSED = 0x00400000,
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 ?

0 comments on commit 8d02f6a

Please sign in to comment.