Skip to content

Commit

Permalink
More fixes regarding FieldBaseOffset vs. the actual field base offset
Browse files Browse the repository at this point in the history
  • Loading branch information
trylek committed Apr 5, 2021
1 parent 2c9bbd4 commit 8db3511
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType
// For types inheriting from another type, field offsets continue on from where they left off
// For reference types, we calculate field alignment as if the address after the method table pointer
// has offset 0 (on 32-bit platforms, this location is guaranteed to be 8-aligned).
LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false) - offsetBias;
LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false, requiresAlignedBase: false) - offsetBias;

var layoutMetadata = type.GetClassLayout();

Expand Down Expand Up @@ -434,7 +434,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType
return computedLayout;
}

protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8)
protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8, bool requiresAlignedBase)
{
}

Expand Down Expand Up @@ -541,7 +541,9 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
LayoutInt offsetBias = OffsetBias(type);

// For types inheriting from another type, field offsets continue on from where they left off
LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8);
// Base alignment is not always required, it's only applied when there's a version bubble boundary
// between base type and the current type.
LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8, requiresAlignedBase: false);
if (!cumulativeInstanceFieldPos.IsIndeterminate)
{
cumulativeInstanceFieldPos -= offsetBias;
Expand Down Expand Up @@ -758,7 +760,7 @@ private static bool IsByValueClass(TypeDesc type)
return type.IsValueType && !type.IsPrimitive && !type.IsEnum;
}

public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8)
public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8, bool requiresAlignedBase)
{
LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero;

Expand All @@ -773,7 +775,7 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8
{
cumulativeInstanceFieldPos += LayoutInt.One;
}
AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8);
AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8, requiresAlignedBase);
cumulativeInstanceFieldPos += offsetBias;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,12 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type)
}
}

public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type, type.RequiresAlign8());
/// <summary>
/// This is a rough equivalent of the CoreCLR runtime method ReadyToRunInfo::GetFieldBaseOffset.
/// In contrast to the auto field layout algorithm, this method unconditionally applies alignment
/// between base and derived class (even when they reside in the same version bubble).
/// </summary>
public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type, type.RequiresAlign8(), requiresAlignedBase: true);

public void SetCompilationGroup(ReadyToRunCompilationModuleGroupBase compilationModuleGroup)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -817,18 +817,13 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada
/// This method decides whether the type needs aligned base offset in order to have layout resilient to
/// base class layout changes.
/// </summary>
protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8)
protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8, bool requiresAlignedBase)
{
DefType baseType = type.BaseType;

if (!_compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)baseType, derivedType: type))
if (requiresAlignedBase || _compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)type.BaseType, derivedType: type))
{
// The type is defined in the module that's currently being compiled and the type layout doesn't depend on other modules
return;
LayoutInt alignment = new LayoutInt(requiresAlign8 ? 8 : type.Context.Target.PointerSize);
baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target);
}

LayoutInt alignment = new LayoutInt(requiresAlign8 ? 8 : type.Context.Target.PointerSize);
baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target);
}

public static bool IsManagedSequentialType(TypeDesc type)
Expand Down

0 comments on commit 8db3511

Please sign in to comment.