[NativeAOT] __GetFieldHelper emitted for simple contiguous struct

[NinoFloris]

Description

__GetFieldHelper emitted for simple contiguous struct.

Reproduction Steps

Some examples:

struct NoHelper
{
    long _one;
    long _two;
}

struct Helper
{
    long _one;
    int _two;
}

struct AlsoNoHelper
{
    long _one;
    int _two;
    int _three;
}

Expected behavior

No __GetFieldHelper emitted for Helper

Actual behavior

Publishing this Helper shows the __GetFieldHelper override in sizoscope.

I'm assuming this is due to the struct alignment introducing padding.
This likely causes the OverlappingFieldTracker to report there are gaps in the struct.

As far as I understand it all padding introduced will be zeroed, so there should be no reason to disable the memcompare implementation.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[jkotas]

As far as I understand it all padding introduced will be zeroed,

That's not correct. Padding in structs is not guaranteed to be zeroed.

[NinoFloris]

Would we at least be able to exclude trailing padding from disabling the optimization?

[MichalStrehovsky]

Would we at least be able to exclude trailing padding from disabling the optimization?

That sounds possible. We just need to solve where to store the number of bytes to look at (we don't have the size-before-alignment available at runtime). Maybe we could use __GetFieldHelper for that. We'd still emit the helper, but it would be much shorter because it would only return the size (as a negative number to let us distinguish).

We currently do this:

runtime/src/coreclr/nativeaot/System.Private.CoreLib/src/System/ValueType.cs

Lines 55 to 60 in 8a2e934

	int numFields = __GetFieldHelper(GetNumFields, out _);
	ref byte thisRawData = ref this.GetRawData();
	ref byte thatRawData = ref obj.GetRawData();
	if (numFields == UseFastHelper)

If __GetFieldHelper returns -1, it means there's no override and just compare memory. Maybe we can change that to test for a negative number. If the negative number is int.MinValue, then consider memory until this.GetMethodTable()->ValueTypeSize as usual, but if the negative number is different, consider memory until -numFields. And if the number is positive, just compare field by field as usual.

[NinoFloris]

Not sure keeping the method around will really help if the cost for these are already in the < 100 byte range. Would be nice if there is a free bit somewhere to specify MethodTable.ValueTypeFieldPadding is purely trailing. Shouldn't that be enough to do a more precise comparison by default?

[jkotas]

MethodTable.ValueTypeFieldPadding is padding added when (boxed) value type is allocated on the GC heap. It is not the user visible padding that you have in mind.

[NinoFloris]

Thanks, I glanced too quickly at ValueTypeSize.