Delete FEATURE_DOUBLE_ALIGNMENT_HINT for GC heap allocations

[jkotas]

This optimization is no longer relevant on current x86 hardware. The performance penalty for misaligned memory access is negligible compared to what it used to be.

Fixes #101284

[Copilot]

Pull Request Overview

This PR removes the FEATURE_DOUBLE_ALIGNMENT_HINT code related to optimizing GC heap allocations for arrays of doubles, since modern x86 hardware no longer suffers from significant performance penalties due to misaligned memory accesses.

• Removed FEATURE_DOUBLE_ALIGNMENT_HINT blocks in helper functions for GC memory allocations.
• Removed configuration settings and related macros associated with the double alignment hint.
• Updated comments to reflect the new behavior for double alignment.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/coreclr/vm/jitinterface.cpp	Removed alignment hint adjustments in GC allocation helpers.
src/coreclr/vm/i386/jitinterfacex86.cpp	Removed threshold adjustments for double arrays for large object heap.
src/coreclr/vm/gchelpers.cpp	Deleted FEATURE_DOUBLE_ALIGNMENT_HINT conditions in allocation routines.
src/coreclr/vm/eeconfig.h	Removed configuration getters and members for the double alignment feature.
src/coreclr/vm/eeconfig.cpp	Deleted initialization and configuration code for double alignment.
src/coreclr/inc/switches.h	Updated comments to reflect changes in double alignment usage.
src/coreclr/inc/clrconfigvalues.h	Removed configuration value for double array to large object heap mapping.

Comments suppressed due to low confidence (1)

src/coreclr/inc/switches.h:154

• The updated comment now focuses on double alignment for structs on the stack, which differs from the previous emphasis on arrays of doubles. Please verify that this comment accurately reflects the intended behavior after removing FEATURE_DOUBLE_ALIGNMENT_HINT.

// Prefer double alignment for structs with doubles on the stack.

[jkotas]

See discussion at EgorBot/runtime-utils#356 (comment)

• Keeping the optimization for stack allocations for now. The impact of removing it for stack allocations can be evaluated separately.
• The implementations of x86 allocation helpers will be deleted in the upcoming change that unifies allocation helpers with NAOT.

[jkotas]

cc @filipnavara

[am11]

@EgorBot -windows_intel -use32bit

using BenchmarkDotNet.Attributes;

public class Bench
{
    [Params(16, 32, 64, 128)]
    public int Size;

    [Benchmark]
    public double[] AllocDoubleArray() => new double[Size];

    [Benchmark]
    public double[] AllocDoubleArrayFixed() => new double[32];
}

[am11]

👍

[am11]

The other usage is in

runtime/src/coreclr/vm/classlayoutinfo.cpp

Line 494 in cbd7849

if (ShouldAlign8(numR8Fields, numInstanceFields))

which can be inlined:
if (numR8Fields * 2 > numInstanceFields && numR8Fields >= 2)
(only usage of ShouldAlign8)

[jkotas]

Yes, it can be inlined but it won't make the code easier to understand. This heuristic is duplicated in AOT compilers in method with the same name:

runtime/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

Line 2259 in 1548409

private static bool ShouldAlign8(int dwR8Fields, int dwTotalFields)

[am11]

My point was FEATURE_DOUBLE_ALIGNMENT_HINT can be replaced by defined(FEATURE_64BIT_ALIGNMENT) || defined(TARGET_32BIT) since the remaining two usages have nothing to do with double alignment.

[jkotas]

The remaining usages have to do with double alignment on stack. I have updated the comment on FEATURE_DOUBLE_ALIGNMENT_HINT to reflected that and mentioned in the comment above that we can evaluate deleting that separately.

[filipnavara]

Thanks!

[MichalStrehovsky]

We may have outerloop tests that test this such as:

runtime/src/tests/JIT/Methodical/doublearray/dblarray1.cs

Lines 4 to 11 in 1548409

	// Goal: Test arrays of doubles are allocated on large object heap and therefore 8 byte aligned
	// Assumptions:
	// 1) large object heap is always 8 byte aligned
	// 2) double array greater than 1000 elements is on large object heap
	// 3) non-double array greater than 1000 elements but less than 85K is NOT on large object heap
	// 4) new arrays allocated in large object heap is of generation 2
	// 5) new arrays NOT allocated in large object heap is of generation 0
	// 6) the threshold can be set by registry key DoubleArrayToLargeObjectHeap

Searching the codebase for 101284 may find those places because we disable these for native AOT and that's the issue number for it.

[jkotas]

/azp run runtime-coreclr outerloop

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[jkotas]

/azp run runtime-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MichalPetryka]

Isn't this still required for correctness on ARM32?

[filipnavara]

Isn't this still required for correctness on ARM32?

ARM32 [and WASM] uses a different define - FEATURE_64BIT_ALIGNMENT.

[MichalPetryka]

Isn't this still required for correctness on ARM32?

ARM32 [and WASM] uses a different define - FEATURE_64BIT_ALIGNMENT.

Ah okay.

The performance penalty for misaligned memory access is negligible compared to what it used to be.

Wasn't there an issue a few months ago where an Intel engineer confirmed that unaligned atomics are very slow on X86 for 64bit xchg/cmpxchg? It might be worth testing how those are affected in perf with this.

[filipnavara]

Wasn't there an issue a few months ago where an Intel engineer confirmed that unaligned atomics are very slow on X86 for 64bit xchg/cmpxchg? It might be worth testing how those are affected in perf with this.

This PR specifically affects the double type. I don't consider it likely that someone would use Interlocked.[Compare]Exchange with a double on x86.

[jkotas]

Wasn't there an issue a few months ago where an Intel engineer confirmed that unaligned atomics are very slow on X86 for 64bit xchg/cmpxchg? It might be worth testing how those are affected in perf with this.

Yes, that's correct. However, Interlocked.* operations are much more likely to be used for long/ulong than for double. long/ulong are misaligned and have this problem like half of the time on x86. This PR is not changing how long/ulong are handled.

[jkotas]

/ba-g infrastructure timeouts and known issues

[filipnavara]

Thanks!