Use WASM CoreLib when compiling crossgen2 WASM unit tests#131024
Draft
jtschuster wants to merge 4 commits into
Draft
Use WASM CoreLib when compiling crossgen2 WASM unit tests#131024jtschuster wants to merge 4 commits into
jtschuster wants to merge 4 commits into
Conversation
- Use CopyAllJitLibrariesToAotCompilerOutput=true instead of copying ourselves. - Set crossgenDir based on crossgen2_inbuild.csproj output items - Reorder properties and items to keep related items close to each other
…ter-wasm-r2r-crossgen-test
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the ReadyToRun crossgen2 test runner and CI wiring so WASM-targeted R2R compilations can reference the browser-wasm runtime pack (including WASM System.Private.CoreLib) when it’s available, instead of always using the host runtime pack.
Changes:
- Add browser-wasm runtime-pack path plumbing (and an opt-in “require wasm references” switch) to the ILCompiler.ReadyToRun.Tests test infrastructure.
- Add a per-compilation target selector (
TargetRid) so tests can request browser-wasm (or host-arm) crossgen2 args and reference sets. - Update pipeline to build + publish browser-wasm managed runtime-pack artifacts and download them into the CLR tools test job.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj | Makes IncludeStaticLibrariesInPack overridable so CI can disable static libs for browser/wasi runtime-pack builds. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/TestPaths.cs | Simplifies path config and adds optional wasm runtime-pack locations + RequireWasmReferences switch. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RTestRunner.cs | Chooses host vs browser-wasm reference paths per compilation and passes reference paths into the resolver. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RTestCaseCompiler.cs | Changes Roslyn compilation reference setup to consume an explicit reference list. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs | Updates SimpleAssemblyResolver to resolve framework assemblies from a provided reference set rather than TestPaths. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RDriver.cs | Removes the dedicated “target arch arm” option in favor of TargetRid-driven args. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs | Updates wasm and arm test cases to use TargetRid and tightens arm test gating. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/ILCompiler.ReadyToRun.Tests.csproj | Adds wasm runtime-pack discovery/config propagation and CI-oriented validation/warnings. |
| eng/pipelines/runtime.yml | Builds browser-wasm libs for CLR tools tests, publishes them as artifacts, and downloads them into the tools test job. |
Comments suppressed due to low confidence (1)
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/TestPaths.cs:8
- System.Text.RegularExpressions is no longer used in this file after removing the config-probing helpers; leaving it will either produce an unnecessary compiler warning (CS8019) or fail builds that treat warnings as errors.
using System;
using System.IO;
using System.Runtime.InteropServices;
using System.Text.RegularExpressions;
using Xunit.Abstractions;
Comment on lines
37
to
41
| /// <summary> | ||
| /// Path to the crossgen2 output directory (contains the self-contained crossgen2 executable and clrjit). | ||
| /// e.g. artifacts/bin/coreclr/linux.x64.Checked/x64/crossgen2/ | ||
| /// Falls back to Checked or Release if the configured path doesn't exist. | ||
| /// </summary> |
Comment on lines
254
to
263
| var paths = new Dictionary<string, string>(); | ||
|
|
||
| foreach (var asm in assemblies) | ||
| { | ||
| // Tests shouldn't require a platform-specific runtime/ref pack for Roslyn compilation | ||
| var defaultReferences = BuildReferencePaths(useWasmReferences: false); | ||
| var compiler = new R2RTestCaseCompiler(defaultReferences); | ||
| var sources = asm.SourceResourceNames | ||
| .Select(R2RTestCaseCompiler.ReadEmbeddedSource) | ||
| .ToList(); |
Comment on lines
74
to
77
| new CSharpCompilationOptions(outputKind) | ||
| .WithOptimizationLevel(OptimizationLevel.Release) | ||
| .WithOptimizationLevel(OptimizationLevel.Debug) | ||
| .WithAllowUnsafe(true) | ||
| .WithNullableContextOptions(NullableContextOptions.Enable)); |
Comment on lines
+52
to
+70
| <Target Name="ValidatePathsExist" | ||
| BeforeTargets="Build" | ||
| Condition="'$(DesignTimeBuild)' != 'true'"> | ||
|
|
||
| <Error Condition="!Exists('$(CoreCLRArtifactsPath)\$(BuildArchitecture)\crossgen2')" | ||
| Text="Crossgen2 inbuild publish directory '$(CoreCLRArtifactsPath)\$(BuildArchitecture)\crossgen2' does not exist. This is a build authoring error and should be generated in the test project build." /> | ||
| <Error Condition="!Exists('$(MicrosoftNetCoreAppRuntimePackNativeDir)')" | ||
| Text="Runtime pack path '$(MicrosoftNetCoreAppRuntimePackNativeDir)' does not exist." /> | ||
| <Error Condition="!Exists('$(MicrosoftNetCoreAppRuntimePackRidLibTfmDir)')" | ||
| Text="Runtime pack path '$(MicrosoftNetCoreAppRuntimePackRidLibTfmDir)' does not exist." /> | ||
|
|
||
| <!-- Wasm runtime pack is not strictly required for local builds, but is enforced in CI with RequireWasmReferences --> | ||
| <Error Condition="'$(RequireWasmReferences)' == 'true' and (!Exists('$(WasmRuntimePackDir)') or !Exists('$(WasmRuntimePackNativeDir)System.Private.CoreLib.dll'))" | ||
| Text="The browser-wasm runtime pack was not found under '$(WasmRuntimePackRoot)'. Build and download the wasm prerequisite artifact before building this test." /> | ||
| <Warning Condition="'$(RequireWasmReferences)' != 'true' and !Exists('$(WasmRuntimePackDir)')" | ||
| Text="Browser-wasm runtime pack path '$(WasmRuntimePackDir)' does not exist. Browser-wasm ReadyToRun tests will fall back to host references, which may cause unexpected failures. Generate by building the clr.corelib+libs+libs.pretest subsets for browser-wasm." /> | ||
| <Warning Condition="'$(RequireWasmReferences)' != 'true' and !Exists('$(WasmRuntimePackNativeDir)System.Private.CoreLib.dll')" | ||
| Text="Browser-wasm System.Private.CoreLib.dll was not found under '$(WasmRuntimePackNativeDir)'. Browser-wasm ReadyToRun tests will fall back to host references, which may cause unexpected failures. Generate by building the clr.corelib+libs+libs.pretest subsets for browser-wasm." /> | ||
| </Target> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description to come...