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.

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

Have CoreCLR follow sequential layout with GC refs #18

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions src/coreclr/clr.featuredefines.props
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<FeatureBasicFreeze>true</FeatureBasicFreeze>
<FeatureGenericMath>true</FeatureGenericMath>
<ProfilingSupportedBuild>true</ProfilingSupportedBuild>
<SequentialLayoutWithRefs>true</SequentialLayoutWithRefs>
</PropertyGroup>

<PropertyGroup Condition="'$(TargetsUnix)' == 'true'">
Expand Down Expand Up @@ -64,5 +65,6 @@

<DefineConstants Condition="'$(ProfilingSupportedBuild)' == 'true'">$(DefineConstants);PROFILING_SUPPORTED</DefineConstants>
<DefineConstants Condition="'$(FeatureProfAttach)' == 'true'">$(DefineConstants);FEATURE_PROFAPI_ATTACH_DETACH</DefineConstants>
<DefineConstants Condition="'$(SequentialLayoutWithRefs)' == 'true'">$(DefineConstants);FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS</DefineConstants>
</PropertyGroup>
</Project>
3 changes: 3 additions & 0 deletions src/coreclr/clrdefinitions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ if (CLR_CMAKE_TARGET_WIN32 AND CLR_CMAKE_TARGET_ARCH_AMD64)
add_definitions(-DFEATURE_SPECIAL_USER_MODE_APC)
endif()

if (FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS)
add_definitions(-DFEATURE_SEQUENTIAL_LAYOUT_WITH_REFS)
endif()

# Use this function to enable building with a specific target OS and architecture set of defines
# This is known to work for the set of defines used by the JIT and gcinfo, it is not likely correct for
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/clrfeatures.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,8 @@ endif()
if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS)
set(FEATURE_OBJCMARSHAL 1)
endif()

# should default this to 0 once we figure out how to enable in build scripting
if(NOT DEFINED FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS)
set(FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS 1)
endif(NOT DEFINED FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS)
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp
type.Context.Target.GetWellKnownTypeAlignment(type),
0,
alignUpInstanceByteSize: true,
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
ignoreLayoutAlignment: false,
#endif
out instanceByteSizeAndAlignment
);

Expand Down Expand Up @@ -302,7 +305,11 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
LayoutInt instanceSize = cumulativeInstanceFieldPos + offsetBias;

var layoutMetadata = type.GetClassLayout();
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
int packingSize = ComputePackingSize(type, layoutMetadata, false);
#else
int packingSize = ComputePackingSize(type, layoutMetadata);
#endif
LayoutInt largestAlignmentRequired = LayoutInt.One;

var offsets = new FieldAndOffset[numInstanceFields];
Expand Down Expand Up @@ -356,6 +363,9 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
largestAlignmentRequired,
layoutMetadata.Size,
alignUpInstanceByteSize: AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(type),
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
ignoreLayoutAlignment: false,
#endif
out instanceByteSizeAndAlignment);

ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
Expand All @@ -376,7 +386,11 @@ private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, Layo
return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target);
}

#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields, bool isSequentialWithRefs)
#else
protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields)
#endif
{
var offsets = new FieldAndOffset[numInstanceFields];

Expand All @@ -390,7 +404,11 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType

LayoutInt largestAlignmentRequirement = LayoutInt.One;
int fieldOrdinal = 0;
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
int packingSize = ComputePackingSize(type, layoutMetadata, ignoreLayoutPacking: isSequentialWithRefs);
#else
int packingSize = ComputePackingSize(type, layoutMetadata);
#endif
bool layoutAbiStable = true;

foreach (var field in type.GetFields())
Expand Down Expand Up @@ -418,6 +436,9 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType
largestAlignmentRequirement,
layoutMetadata.Size,
alignUpInstanceByteSize: true,
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
ignoreLayoutAlignment: isSequentialWithRefs,
#endif
out instanceByteSizeAndAlignment);

ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
Expand All @@ -442,7 +463,11 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
bool hasLayout = type.HasLayout();
var layoutMetadata = type.GetClassLayout();

#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
int packingSize = ComputePackingSize(type, layoutMetadata, false);
#else
int packingSize = ComputePackingSize(type, layoutMetadata);
#endif
packingSize = Math.Min(context.Target.MaximumAutoLayoutPackingSize, packingSize);

var offsets = new FieldAndOffset[numInstanceFields];
Expand Down Expand Up @@ -706,6 +731,9 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
minAlign,
classLayoutSize: 0,
alignUpInstanceByteSize: true,
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
ignoreLayoutAlignment: false,
#endif
out instanceByteSizeAndAlignment);

ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
Expand Down Expand Up @@ -828,15 +856,27 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType,
return result;
}

#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata layoutMetadata, bool ignoreLayoutPacking)
#else
private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata layoutMetadata)
#endif
{
if (layoutMetadata.PackingSize == 0)
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
if (layoutMetadata.PackingSize == 0 || ignoreLayoutPacking)
#else
if (layoutMetadata.PackingSize == 0)
#endif
return type.Context.Target.DefaultPackingSize;
else
return layoutMetadata.PackingSize;
}

#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, bool alignUpInstanceByteSize, bool ignoreLayoutAlignment, out SizeAndAlignment byteCount)
#else
private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, bool alignUpInstanceByteSize, out SizeAndAlignment byteCount)
#endif
{
SizeAndAlignment result;

Expand All @@ -862,6 +902,16 @@ private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt
}
else
{
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
if (ignoreLayoutAlignment)
{
// Even if we want to ignore layout alignment, the GC requires types with GC refs to be
// sized to a multiple of LayoutPointerSize
if (type.ContainsGCPointers)
instanceSize = LayoutInt.AlignUp(instanceSize, target.LayoutPointerSize, target);
}
else
#endif
if (type.IsValueType)
{
instanceSize = LayoutInt.AlignUp(instanceSize,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -803,10 +803,17 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada
return ComputeExplicitFieldLayout(type, numInstanceFields);
}
else
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
if (ShouldLayoutSequential(type, out var isSequentialWithRefs))
{
return ComputeSequentialFieldLayout(type, numInstanceFields, isSequentialWithRefs);
}
#else
if (type.IsEnum || MarshalUtils.IsBlittableType(type) || IsManagedSequentialType(type))
Copy link

Choose a reason for hiding this comment

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

Note that this logic got a major overhaul in dotnet#61759 . I think you will want to do matching cleanup of FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS mode once you pick up that change. I hope that it will make things simpler overall.

Choose a reason for hiding this comment

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

Thanks for letting us know about this pull request, it looks very useful!

{
return ComputeSequentialFieldLayout(type, numInstanceFields);
}
#endif
else
{
return ComputeAutoFieldLayout(type, numInstanceFields);
Expand Down Expand Up @@ -865,5 +872,58 @@ public static bool IsManagedSequentialType(TypeDesc type)

return true;
}

#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
private static bool ShouldLayoutSequential(TypeDesc type, out bool isSequentialWithRefs)
{
isSequentialWithRefs = false;

var name = type.GetDisplayName();

if (type.IsEnum)
{
return true;
}

if (MarshalUtils.IsBlittableType(type))
{
return true;
}

if (IsManagedSequentialType(type))
{
return true;
}

isSequentialWithRefs = HasValidSequentialLayout(type);
return isSequentialWithRefs;
}

private static bool HasValidSequentialLayout(TypeDesc type)
{
var metadataType = type as MetadataType;
if (metadataType == null)
{
return false;
}

if (!metadataType.IsSequentialLayout)
{
return false;
}

if (!metadataType.HasBaseType)
{
return false;
}

if (metadataType.BaseType.IsObject || metadataType.BaseType.IsWellKnownType(WellKnownType.ValueType))
{
return true;
}

return HasValidSequentialLayout(type.BaseType);
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
<Configurations>Debug;Release;Checked</Configurations>
</PropertyGroup>

<!-- Compilation options -->
<Import Project="../../../clr.featuredefines.props" />


<ItemGroup>
<ProjectReference Include="..\ILCompiler.DependencyAnalysisFramework\ILCompiler.DependencyAnalysisFramework.csproj" />
<ProjectReference Include="..\ILCompiler.Diagnostics\ILCompiler.Diagnostics.csproj" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>

<!-- Compilation options -->
<Import Project="../../../clr.featuredefines.props" />

<ItemGroup>
<PackageReference Include="xunit.core" Version="$(XUnitVersion)" ExcludeAssets="build" />
<PackageReference Include="Microsoft.DotNet.XUnitExtensions" Version="$(MicrosoftDotNetXUnitExtensionsVersion)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada
}
else if (type.IsSequentialLayout || type.IsEnum)
{
#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
return ComputeSequentialFieldLayout(type, numInstanceFields, false);
#else
return ComputeSequentialFieldLayout(type, numInstanceFields);
#endif
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
<Configurations>Debug;Release;Checked</Configurations>
</PropertyGroup>

<!-- Compilation options -->
<Import Project="../../../clr.featuredefines.props" />

<ItemGroup Label="Embedded Resources">
<EmbeddedResource Include="..\..\Common\TypeSystem\Common\Properties\Resources.resx">
<LogicalName>Internal.TypeSystem.Strings.resources</LogicalName>
Expand Down
30 changes: 30 additions & 0 deletions src/coreclr/vm/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,11 @@ class EEClassLayoutInfo
e_ZERO_SIZED = 0x04,
// The size of the struct is explicitly specified in the meta-data.
e_HAS_EXPLICIT_SIZE = 0x08

#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
// The type has GC references, but respect sequential layout
, e_SEQUENTIAL_WITH_REFS = 0x10
#endif
};

BYTE m_bFlags;
Expand Down Expand Up @@ -419,6 +424,14 @@ class EEClassLayoutInfo
return (m_bFlags & e_HAS_EXPLICIT_SIZE) == e_HAS_EXPLICIT_SIZE;
}

#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
BOOL IsSequentialWithRefs() const
{
LIMITED_METHOD_CONTRACT;
return (m_bFlags & e_SEQUENTIAL_WITH_REFS) == e_SEQUENTIAL_WITH_REFS;
}
#endif

BYTE GetPackingSize() const
{
LIMITED_METHOD_CONTRACT;
Expand Down Expand Up @@ -453,6 +466,13 @@ class EEClassLayoutInfo
m_bFlags = hasExplicitSize ? (m_bFlags | e_HAS_EXPLICIT_SIZE)
: (m_bFlags & ~e_HAS_EXPLICIT_SIZE);
}

void SetIsSequentialWithRefs(BOOL isSequenialWithRefs)
{
LIMITED_METHOD_CONTRACT;
m_bFlags = isSequenialWithRefs ? (m_bFlags | e_SEQUENTIAL_WITH_REFS)
: (m_bFlags & ~e_SEQUENTIAL_WITH_REFS);
}
};

//
Expand Down Expand Up @@ -1382,6 +1402,10 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW!

BOOL HasExplicitSize();

#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
BOOL IsSequentialWithRefs();
#endif

static void GetBestFitMapping(MethodTable * pMT, BOOL *pfBestFitMapping, BOOL *pfThrowOnUnmappableChar);

/*
Expand Down Expand Up @@ -2062,6 +2086,12 @@ inline BOOL EEClass::HasExplicitSize()
return HasLayout() && GetLayoutInfo()->HasExplicitSize();
}

inline BOOL EEClass::IsSequentialWithRefs()
{
LIMITED_METHOD_CONTRACT;
return HasLayout() && GetLayoutInfo()->IsSequentialWithRefs();
}

//==========================================================================
// These routines manage the prestub (a bootstrapping stub that all
// FunctionDesc's are initialized with.)
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/vm/classlayoutinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,16 @@ VOID EEClassLayoutInfo::CollectLayoutFieldMetadataThrowing(
}

pEEClassLayoutInfoOut->SetIsManagedSequential(!fDisqualifyFromManagedSequential);

#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
BOOL fIsSequentialWithRefs = FALSE;
if (fDisqualifyFromManagedSequential && !fExplicitOffsets)
{
fIsSequentialWithRefs = !fHasNonTrivialParent || (fParentHasLayout && (pParentLayoutInfo->IsSequentialWithRefs() || pParentLayoutInfo->IsManagedSequential() || pParentLayoutInfo->IsBlittable()));
}

pEEClassLayoutInfoOut->SetIsSequentialWithRefs(fIsSequentialWithRefs);
#endif
}

void EEClassNativeLayoutInfo::InitializeNativeLayoutFieldMetadataThrowing(MethodTable* pMT)
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/managedmdimport.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ typedef struct
I4Array * largeResult;
int length;
#ifdef HOST_64BIT
#ifndef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS
int padding;
#endif
#endif
int smallResult[16];
} MetadataEnumResult;
Expand Down
Loading