CORINFO_FLG_CUSTOMLAYOUT semantics

[jkotas]

CORINFO_FLG_CUSTOMLAYOUT flag on the JIT-EE interface has unclear semantics and it is likely disabling optimizations for no good reason in some cases. We should precisely define the semantics of this change and change the implementation to match. Depending on the definition, rename of the flag may be appropriate too.

• The flag is set for auto-layout structs. It should not be necessary. (See the original change Don't set CORINFO_FLG_CUSTOMLAYOUT for auto-layout structs #71673.)
• The flag is set for TypedReference. TypedReference is ordinary struct with two fields. It should not be needed.

category:cq
theme:jit-ee-interface
skill-level:expert
cost:small
impact:small

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

CORINFO_FLG_CUSTOMLAYOUT flag on the JIT-EE interface has unclear semantics and it is likely disabling optimizations for no good reason in some cases. We should precisely define the semantics of this change and change the implementation to match. Depending on the definition, rename of the flag may be appropriate too.

• The flag is set for auto-layout structs. It should not be necessary. (See the original change Don't set CORINFO_FLG_CUSTOMLAYOUT for auto-layout structs #71673.)
• The flag is set for TypedReference. TypedReference is ordinary struct with two fields. It should not be needed.

Author:	jkotas
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

cc @AaronRobinsonMSFT @EgorBo

[EgorBo]

Also, related: #64863

[SingleAccretion]

Proposed semantics: the flag is set for a struct if the padding in it can be significant to the user.

1. The struct had Size or explicit layout in the metadata.
2. And the struct has "holes".

The proposed name would be CORINFO_FLG_HAS_SIGNIFICANT_PADDING.

Structs with this flag set would not be promotable - this is unlike today, where we do promote them, but then pessimize some "decomposition" transforms, but not all.

[tannergooding]

What would that mean for something like:

[StructLayout(LayoutKind.Explicit)]
public struct S
{
    [FieldOffset(0)]
    public ulong Bits;

    [FieldOffset(0)]
    public double Value;
}

This has explicit layout, but no "holes". So this would still allow promotion if only one of the two fields was accessed?

A union of say a double and float however would have SIGNIFICANT_PADDING even if only one field was accessed?

---

This means that "small" (1-4 element) fixed-sized buffers of primitive types always have "holes" given they currently have 1 field (of the primitive type) and are explicitly sized otherwise?

It may mean we want special semantics for tracking "fixed sized buffers" in the future (there discussions on how to remove or reduce metadata bloat from declaring n fields if the feature becomes more prominent and usable in "safe" code).

[SingleAccretion]

What about unions?

Currently unions are never promoted, so the behavior for them would not change.

This means that "small" (1-4 element) fixed-sized buffers of primitive types always have "holes" given they currently have 1 field (of the primitive type) and are explicitly sized otherwise?

Correct.

[tfenise]

Proposed semantics: the flag is set for a struct if the padding in it can be significant to the user.

1. The struct had Size or explicit layout in the metadata.
2. And the struct has "holes".

Consider:

[StructLayout(LayoutKind.Explicit)]
struct Struct1
{
    [FieldOffset(0)]
    int X;

    [FieldOffset(4)]
    byte Y;
}

and

[StructLayout(LayoutKind.Sequential)]
struct Struct2
{
    int X;

    byte Y;
}

Are Struct1 and Struct2 to be treated differently? They appear to be equivalent.
Can the padding in Struct2 really be considered not "significant to the user"? If the padding in Struct1 is considered significant, and Struct1 and Struct2 are basically equivalent, then the padding in Struct2 should also be.

[SingleAccretion]

Can the padding in Struct2 really be considered not "significant to the user"?

It is a choice to not consider it significant. It would be prohibitively expensive to disqualify structs like Struct2 from promotion, e. g. Span<T> would be non-promotable under such rules.

It does create an asymmetry like you highlight. I do not see an easy way out of that. Considering padding in explicitly laid out structs insignificant seems like too dangerous a breaking change.