Add ThisptrRetbufPrecode to set of precodes handled by StubPrecode path

[davidwrighton]

This adds the ThisPtrRetBufPrecode to the set of codes handled by the StubPrecode stub pasting path.

Perf changes from about 2.35 ns per iteration of a minimal loop with a ThisPtrRetBufPrecode to about 3.03 ns per iteration on my computer (Windows X64). I expect the cost to be a little less on an Arm64 machine since the old ThisPtrRetBufPrecode was a bit more expensive there. However, this removes the previous scheme where we allocate and jit custom stubs for this, so I think the simplification is worth the runtime cost.

The overall change has a size increase though, as this work adds support for this type of precode to the cDAC, as well as some tests.

Test app

 using System.Diagnostics;

namespace ThisPtrRetBufPerfTest
{

    internal class Program
    {
        public struct StructThatNeedsRetBuf
        {
            long x;
            long y;
            long z;
        }

        public static StructThatNeedsRetBuf TakesAnObject(object obj)
        {
            return default(StructThatNeedsRetBuf);
        }

        public static void TestDelegate(int count, Func<StructThatNeedsRetBuf> del)
        {
            Stopwatch sw = Stopwatch.StartNew();
            for (int i = 0; i < count; i++)
            {
                del();
            }
            sw.Stop();
            Console.WriteLine($"Time for {count} iterations {sw.ElapsedMilliseconds}ms");
        }

        static void Main(string[] args)
        {
            object o = new Program();

            Func<StructThatNeedsRetBuf> aDelegate = typeof(Program).GetMethod(nameof(TakesAnObject))!.CreateDelegate<Func<StructThatNeedsRetBuf>>(o);

            TestDelegate(1000, aDelegate);
            TestDelegate(100_000_000, aDelegate);
            TestDelegate(100_000_000, aDelegate);
            TestDelegate(100_000_000, aDelegate);
            TestDelegate(100_000_000, aDelegate);
        }
    }
}

[dotnet-policy-service]

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

[Copilot]

Pull Request Overview

This PR adds support for handling the ThisPtrRetBufPrecode in the StubPrecode path by updating both the design documentation and the precode resolution logic.

• Updated design doc to include Version 2 of the precode contract with ThisPtrRetBufPrecode support.
• Modified the GetMethodDesc methods to use different offsets based on the precode contract version.

Comments suppressed due to low confidence (2)

docs/design/datacontracts/PrecodeStubs.md:27

• [nitpick] There is a naming inconsistency between the PR title ('ThisPtrRetBufPrecode') and the documentation ('ThisPointerRetBufPrecodeType'). Consider aligning the naming for consistency.

| PrecodeMachineDescriptor | ThisPointerRetBufPrecodeType | precode sort byte for this pointer ret buf precodes |

docs/design/datacontracts/PrecodeStubs.md:180

• The branch for ContractVersion(PrecodeStubs) == 1 in this implementation throws a NotImplementedException, which could lead to runtime errors if this code path is hit. Consider adding a clarifying comment or an appropriate fallback to ensure production stability.

throw new NotImplementedException(); // TODO(cdac)

[janvorli]

LGTM modulo two nits.

[janvorli]

A nit - use the ThisPtrRetBufPrecodeData__Target instead of 0 like in the Unix code?

[janvorli]

A nit - this comment would be worth updating to mention the new precode

[max-charlamb]

Maybe we could re-use these existing code from PrecodeStubs_1.

This would require a an extra file, but I think eventually it may make sense to have a folder and namespace for each contract that has helpers or multiple versions.

[max-charlamb]

I'm wondering if it makes more sense to write tests for each feature instead of for each contract version. On contracts versions that don't support a feature, the test can be skipped.

For example have the existing test run on both contract versions then the ThisPtrRetBufPrecodeEntry only run on version 2.

[am11]

@davidwrighton browser-wasm was failing the build when it was merged:

  /__w/1/s/src/coreclr/classlibnative/../vm/precode.h:268:8: error: redefinition of 'ThisPtrRetBufPrecode'
    268 | struct ThisPtrRetBufPrecode : StubPrecode
        |        ^
  /__w/1/s/src/coreclr/classlibnative/../vm/wasm/cgencpu.h:53:8: note: previous definition is here
     53 | struct ThisPtrRetBufPrecode {
        |        ^