Add support for composite r2r runs

[BrzVlad]

This is achieved by using already existing properties in the sdk: PublishReadyToRun and PublishReadyToRunComposite. The compositer2r toolchain adds these properties to a standard project template. In order to test with custom built runtime, the toolchain reuses the already existing customruntimepack and aotcompilerpath bdn args.

Example command used locally from microbenchmarks project:

dotnet run -c Release -f net10.0 --runtimes r2r10_0 --filter "System.Tests.Perf_Int32.TryFormat" --customruntimepack /home/vbrezae/runtime3/artifacts/bin/microsoft.netcore.app.runtime.linux-arm64/Release/ --aotcompilerpath /home/vbrezae/runtime3/artifacts/packages/Release/Shipping/crossgen2pack/

[timcassell]

There has been user interest in a R2R toolchain (#2514). I think it would be good to make it work for general use, not just custom runtime. Could also make "Composite" an optional setting.

[BrzVlad]

There has been user interest in a R2R toolchain (#2514). I think it would be good to make it work for general use, not just custom runtime. Could also make "Composite" an optional setting.

@timcassell I didn't really make it clear, but it actually works already with a normal sdk. If there is no passed custom runtime pack and crossgen2 pack then the TrickRuntimePackLocation target has no effect.

On a separate note, after rebasing to latest main, I tried using the locally built bdn with a sample project. The autogenerated project failed to build with the following error:

/home/vbrezae/.dotnet/sdk/10.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.CrossGen.targets(467,5): error : Unhandled exception. System.CommandLine.CommandLineException: Multiple input files matching same simple name /home/vbrezae/Xamarin/tests/bdn-test/bdn-proj/bin/Release/net10.0/bdn-test-CompositeR2R-1/obj/BenchmarkDotNet.Autogenerated/release_net10.0_linux-arm64/bdn-test-CompositeR2R-1.dll /home/vbrezae/Xamarin/tests/bdn-test/bdn-proj/bin/Release/net10.0/bdn-test-CompositeR2R-1/bin/Release/net10.0/linux-arm64/publish/bdn-test-CompositeR2R-1.dll [/home/vbrezae/Xamarin/tests/bdn-test/bdn-proj/bin/Release/net10.0/bdn-test-CompositeR2R-1/BenchmarkDotNet.Autogenerated.csproj::TargetFramework=net10.0]

This error is triggered by crossgen2 when it receives the same assembly twice as an input. As you can see in the error message, it receives the bdn-test-CompositeR2R-1.dll twice. One usage comes from the obj folder which is suspicious. After a bit of bisecting, turns out that this started happening after 9368a7d. Would you be able to take a look at this ?

[timcassell]

That sounds like the same issue as #2931 (comment). Thanks for finding the problematic commit. I'll take a look when I get some time.

[BrzVlad]

@timcassell Thanks!! #2935 did indeed fix the issue.

Regarding the simple R2R support, does it sound good to rename the toolchain/moniker to ex. R2R10_0 and have an additional --composite bdn arg that enables composite r2r ?

cc @adamsitnik

[timcassell]

I do think it should be renamed, and add public static readonly IToolchain Net10_0 etc fields like other toolchains, and add versions at least as far back as current supported runtimes (net8.0), optionally as far back as R2R was supported (netcoreapp3.0 according to CoPilot, but optional since users could manually construct the toolchain for the old runtimes if they need it).

I think we don't actually need a composite option in the toolchain, just always have it enabled. Users can use MsBuildArgument to override it if they want.

[timcassell]

Also please add integration tests.

[timcassell]

Also I think you should add public class R2RRuntime : CoreRuntime so it will have the correct RuntimeMoniker. And update ToolchainExtensions.GetToolchain to recognize it.

[BrzVlad]

@timcassell I believe I've addressed all your comments. The newly added test is passing now, it is only testing the build step, there is no check at runtime. We could probably check the existence of some r2r binaries but I don't think it is really worth it and it would just make it susceptible to false positive failures in the future.

[timcassell]

Thanks @BrzVlad