Skip to content

Port runtime PR dotnet/runtime#133111 R2R changes to SDK Crossgen tasks - #56395

Merged
maraf merged 9 commits into
mainfrom
maraf-compare-crossgen2-tasks
Oct 2, 2026
Merged

maraf merged 9 commits into
mainfrom
maraf-compare-crossgen2-tasks

Conversation

@maraf

@maraf maraf commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Why

dotnet/runtime's src/tasks/Crossgen2Tasks is a simplified fork of the SDK's shared Crossgen2/ReadyToRun task implementation. Runtime PR dotnet/runtime#133111 ("Use SDK pipeline for WebAssembly framework R2R") made changes to the shared task logic that were never migrated back into this SDK repo. This PR ports the task-and-target portions that apply to the SDK, excluding runtime-only WebCIL and runtime-pack policy.

What changed

  • Microsoft.NET.CrossGen.targets: _CreateR2RImages now tracks @(CrossgenTool), @(Crossgen2Tool), and @(_ReadyToRunCompilerInputs) as incremental inputs, so R2R images are recompiled when the compiler or its dependencies change. Both R2R compiler invocations also forward the runtime-private $(_PublishReadyToRunCrossgen2ExtraArgs) property alongside the existing public arguments.
  • PrepareForReadyToRunCompilation.cs: the R2R output-path helper now supports WebAssembly output. Separately compiled WASM images and composite component stubs use the .wasm paths emitted by Crossgen2, and a composite owner is published as <entry>.r2r.wasm.
  • Existing SDK-specific Mach-O behavior is preserved: separately compiled images use intermediate .o output linked into a published .dylib, while Mach-O composite components retain their managed assembly identity and do not receive native-link metadata.

Testing

Added focused PrepareForReadyToRunCompilation unit coverage for:

  • WASM output and publish paths for separately compiled images
  • WASM composite owner and component-stub paths
  • Mach-O native-link metadata for separately compiled images
  • Unchanged Mach-O composite component paths and metadata

Local validation:

  • Full Debug repository build completed successfully with 0 warnings and 0 errors.
  • Created and published a Blazor WebAssembly sample with the built 11.0.100-dev SDK, UseMonoRuntime=false, and PublishReadyToRun=true.
  • The captured publish binlog confirms the locally built Microsoft.NET.Build.Tasks.dll ran PrepareForReadyToRunCompilation and produced 37 .wasm paths in both _ReadyToRunCompileList and _ReadyToRunFilesToPublish.

Note

This PR description was generated with the assistance of GitHub Copilot.

Bring the SDK's shared Crossgen2/ReadyToRun task implementation in sync
with the equivalent runtime-side changes from dotnet/runtime PR #133111
("Use SDK pipeline for WebAssembly framework R2R"):

- Track CrossgenTool, Crossgen2Tool, and the new
  @(_ReadyToRunCompilerInputs) item as incremental inputs to
  _CreateR2RImages, so R2R images are recompiled when the compiler or
  its dependencies change.
- Forward the runtime-private $(_PublishReadyToRunCrossgen2ExtraArgs)
  property alongside the existing public extra-args property on both
  R2R compiler invocations.
- Extend R2R output-path calculation to support WebAssembly output
  (.wasm), which Crossgen2 now emits directly, while preserving the
  existing Mach-O intermediate (.o) to final (.dylib) linking behavior
  for assemblies that are compiled separately.

Composite component assemblies keep their original publish identity
(no container-format extension rewriting or native-link metadata),
matching pre-existing SDK behavior; only images that are compiled
separately go through the container-aware output-path helper.

Added focused regression tests covering WASM output/publish paths,
preserved Mach-O native-link metadata for separately compiled images,
unaffected composite component paths, and the new target-level
incremental input/argument wiring.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
2 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@maraf maraf self-assigned this Sep 24, 2026
@maraf maraf added this to the 12.0-preview1 milestone Sep 24, 2026
@github-actions

This comment has been minimized.

@lewing

lewing commented Sep 24, 2026

Copy link
Copy Markdown
Member

dotnet/runtime#134618 enables PublishReadyToRunComposite for CoreCLR browser-wasm and depends on the composite owner path this PR introduces: with Crossgen2ContainerFormat=wasm, the owner must be compiled and published as <entry>.r2r.wasm. Crossgen2 records that exact name in each component stub, and the browser loader identifies and probes the owner by it. On current SDK main the owner would come out as <entry>.r2r.dll, so composite browser publishing only works through runtime's Crossgen2Tasks override today.

The added tests cover separately compiled wasm images and Mach-O composite components, but not the wasm composite owner. Could you add a case such as composite: true with "wasm" asserting that the owner's OutputR2RImage, ItemSpec, and RelativePath end in <entry>.r2r.wasm with no RequiresNativeLink, and that component images keep their original identity? That would keep this contract from regressing.

Note

This comment was generated with GitHub Copilot assistance.

@maraf

maraf commented Sep 25, 2026

Copy link
Copy Markdown
Member Author

@copilot apply feedback #56395 (comment)

Copilot AI and others added 3 commits September 25, 2026 12:23
Co-authored-by: maraf <10020471+maraf@users.noreply.github.com>
Co-authored-by: maraf <10020471+maraf@users.noreply.github.com>
Co-authored-by: maraf <10020471+maraf@users.noreply.github.com>

Copilot AI commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

[!WARNING]
Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

...

Fixed in 37f4fbc. The XML assertions now handle nullable values safely and report missing required attributes clearly; the focused test passes.

@copilot apply feedback #56395 (comment)

Applied the linked build-feedback fix in 37f4fbc. The focused ReadyToRun task test now passes.

Preserve forward slashes from RelativePath metadata in cross-platform assertions and verify the composite WASM owner is published without native linking.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@lewing lewing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wasm composite owner test added in 0ad552b covers my earlier comment, thanks. One remaining question inline about behavior differing from dotnet/runtime#133111.

Note

This review was generated with GitHub Copilot assistance.

Comment thread src/Tasks/Microsoft.NET.Build.Tasks/PrepareForReadyToRunCompilation.cs Outdated
Apply container-aware output naming to all WebAssembly inputs so composite component publish items match the .wasm stubs emitted by Crossgen2, while preserving SDK-specific Mach-O behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@maraf

maraf commented Sep 29, 2026

Copy link
Copy Markdown
Member Author
Image

Keep the task tests focused on unit behavior and stop copying the shipping targets file into the test output.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@maraf
maraf marked this pull request as ready for review September 29, 2026 08:23
@maraf
maraf requested a review from a team as a code owner September 29, 2026 08:23
@maraf
maraf requested review from jkoritzinsky and pavelsavara and a balanced review from Copilot September 29, 2026 08:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The incremental target behavior lacks an automated MSBuild-level regression test.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Ports WebAssembly ReadyToRun behavior from the runtime fork into the SDK’s shared Crossgen pipeline.

Changes:

  • Adds WASM output-path handling while preserving Mach-O behavior.
  • Tracks compiler inputs for incremental R2R builds and forwards private compiler arguments.
  • Adds focused task-level path and metadata tests.
File Description
PrepareForReadyToRunCompilation.cs Computes WASM and Mach-O compiler/publish paths.
Microsoft.NET.CrossGen.targets Adds incremental inputs and private Crossgen2 arguments.
GivenAPrepareForReadyToRunCompilation.cs Tests WASM and Mach-O task outputs.

@maraf
maraf requested review from lewing and a balanced review from Copilot September 29, 2026 09:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

WASI composite output naming is incompatible with its consumer, and incremental invalidation lacks automated coverage.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)
Resolved since last review (1)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@lewing

This comment has been minimized.

@lewing

This comment has been minimized.

lewing added a commit to dotnet/runtime that referenced this pull request Oct 1, 2026
## Why

CoreCLR WASI composite ReadyToRun currently names the composite owner
image `composite-r2r.wasm`. `PrepareForReadyToRunCompilation` in
Crossgen2Tasks forces that name whenever `Crossgen2Tool` TargetOS is
`wasi`. The browser composite work in #134618 uses the task's default
name instead: `<entry>.r2r.wasm`, the main assembly name with
`.r2r.wasm`.

This PR drops the TargetOS special case so WASI gets the same owner name
as browser. It lands ahead of the dotnet/sdk#56395 port, so that PR
doesn't carry the WASI rule into the SDK.

The two delivery models stay different on purpose. Browser instantiates
the composite at runtime through the JS loader. WASI composes it offline
into the host with `wasm-merge`/`wasm-opt`. Only the naming contract
changes.

## Design

The composite's own file name, as written by crossgen2, is the single
authority. It is also the owner name every component stub records.

- **`PrepareForReadyToRunCompilation`:** the `wasi` branch is removed. A
wasm composite owner is now `<entry>.r2r.wasm` on both targets.
- **`WasiApp.CoreCLR.targets`:** the composite path now comes from the
planned composite compilation, `@(_ReadyToRunCompileList)` with
`CreateCompositeImage=true`, via `%(OutputR2RImage)`. The build errors
unless there is exactly one composite compilation. Because the path
isn't hardcoded, WASI keeps working with either SDK naming.
- **`wasi_r2r_probe.hpp`:** the host reserves a 256-byte composite name
buffer, zero-initialized, and exports its address and capacity
(`wasi_r2r_composite_name_base`, `wasi_r2r_composite_name_cap`). The
probe serves the composite only when the requested name matches the
recorded name, and only if that name is non-empty. An uncomposed host
therefore serves no composite. There is no wildcard matching and no name
compiled into C++.
- **`ComposeWasiReadyToRun`:** the composer reads the two new exports
and checks that the name is non-empty, contains no NUL, and fits the
buffer. Its shim now imports `webcil.memory` and includes an active data
segment that writes `Path.GetFileName(CompositePath)`, NUL-terminated,
into the host buffer. This is the same mechanism that already installs
the payload. A prebuilt host, such as the runtime-test corerun, can
therefore be composed with a composite of any name.
- **Docs:** updated the WASI host composition section of
`docs/design/mono/webcil.md` and
`docs/workflow/building/coreclr/wasi-r2r.md`.

## Validation

- `./build.sh clr+libs+host+packs -os wasi -arch wasm -c Release`
finished with 0 warnings and 0 errors.
- `./dotnet.sh publish
src/mono/sample/wasi/console/Wasi.Console.Sample.csproj -c Release
-p:TargetOS=wasi -p:TargetArchitecture=wasm -p:RuntimeFlavor=CoreCLR
-p:PublishReadyToRun=true` succeeded. The composer logged
`composite='Wasi.Console.Sample.r2r.wasm'`.
- The published app ran under wasmtime with `DOTNET_ReadyToRunLogFile`
set. It exited 0, and the log shows `Ready to Run initialized
successfully` for System.Private.CoreLib, Wasi.Console.Sample,
System.Runtime, System.Console, System.Threading and
System.Runtime.InteropServices.
- **Negative check:** I composed the same host with a copy of the
composite renamed to `Wrong.r2r.wasm`. Startup then traps in `EEStartup`
because CoreLib's owner composite isn't served. A name mismatch fails
loudly rather than silently interpreting.
- **Non-R2R publish** of the same sample: the app runs on the
interpreter, and the log shows `Ready to Run header not found`.
- **Not run:** browser `ReadyToRunTests`. The removed code only executed
when TargetOS was `wasi`, and the browser naming path is unchanged.

## Interaction with #134813

This PR merges cleanly with #134813's head (checked with `git
merge-tree`); it doesn't depend on #134813. The runtime-test harness in
#134813 passes the same `composite-r2r.wasm` path to both crossgen2 and
`WasiR2RComposer.proj`. The composer records that name in the shared
corerun, so the harness needs no changes. I did not build or run the
runtime tests with both PRs merged.

## Follow-up in dotnet/sdk#56395

- Remove the `wasi` TargetOS block from `CreateReadyToRunFileToPublish`.
- Change `It_uses_the_fixed_wasm_path_for_a_wasi_composite_owner` to
expect `<entry>.r2r.wasm`, the same as browser.

> [!NOTE]
> This pull request description was generated with assistance from
GitHub Copilot.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
lewing added a commit to dotnet/runtime that referenced this pull request Oct 1, 2026
## Summary

- Enable `PublishReadyToRunComposite` for CoreCLR browser-wasm when
WebCIL is enabled. Composite is opt-in; the default per-assembly R2R
mode is unchanged.
- Publish the composite owner (`<entry>.r2r.wasm`, the name crossgen2
records in each component stub) as a static web asset with a
`readyToRunComposite` trait, defined through `DefineStaticWebAssets`.
- `GenerateWasmBootJson` routes the owner to `resources.coreAssembly`
and flags it `isCompositeImage: true`. The loader keeps its `.wasm`
virtual path and leaves it out of the TPA, and component stubs stay in
the TPA. Owner identity comes from the flag, not the file name, so an
ordinary library named `*.r2r` still loads as a managed assembly.
- Route composite component stubs through `ConvertDllsToWebcil`, keep
the linked R2R closure across repeated trimmed publishes, and prune
stale per-app images and stubs.
- Invalidate per-app R2R images when the composite mode or crossgen2
arguments change. Component stubs share the per-assembly `<name>.wasm`
names, so without this, switching composite to per-assembly shipped
stale stubs that failed at startup.
- Composite needs ReadyToRun tasks that name the owner
`<entry>.r2r.wasm`. The base SDK doesn't yet (dotnet/sdk#56395), so a
composite publish with stock SDK tasks fails fast with an actionable
error. Wasm.Build.Tests use the in-repo Crossgen2Tasks, which now also
ship in the `BuildWasmApps` Helix payload.

## Validation

On `main` as of 2026-09-29, including WebCIL wrapper v2 (#134312), WASI
composite in the shared Crossgen2Tasks (#133265), and the browser R2R
strip defaults (#134690):

- Rebuilt the CoreCLR browser Release runtime pack (`clr+libs+host`) and
the WebAssembly SDK pack.
- `Wasm.Build.Tests.ReadyToRunTests`: 10/10 passed. This covers:
  - composite trimmed, untrimmed and native-relinked, run in Chrome
  - a repeated trimmed publish
  - a composite → per-assembly → composite switch
  - per-assembly publish, native relink and dev-loop build
  - composite without WebCIL (negative)
- Negative checks:
- With the composite mode removed from the invalidation stamp, the
mode-switch assertion fails.
- With stock SDK ReadyToRun tasks, composite publish hits the new error.
- CI: `PublishRunAllPagesComposite` passed on Helix on Linux and
Windows. The remaining failures matched known issues (#133953, #133373).

Benchmark (`IndexOfMax<double>(3079)`, browser, earlier revision of this
PR): composite R2R ran the SIMD path with a median of **4,040 ns/op**,
vs. **1,013,940 ns/op** interpreted and **10,652,060 ns/op** with
per-assembly R2R.

Part of testing #134559. That issue's default per-assembly
scenario is not changed by this PR.

> [!NOTE]
> This pull request description was generated with GitHub Copilot
assistance.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@maraf
maraf enabled auto-merge (squash) October 2, 2026 11:43
@maraf
maraf merged commit 8c2c860 into main Oct 2, 2026
23 checks passed
@maraf
maraf deleted the maraf-compare-crossgen2-tasks branch October 2, 2026 16:15
@maraf
maraf deployed to copilot-pat-pool October 2, 2026 16:15 — with GitHub Actions Active
@maraf
maraf deployed to copilot-pat-pool October 2, 2026 16:15 — with GitHub Actions Active

This branch was successfully deployed

1 active deployment
copilot-pat-pool — 1d1452e9 Deployed Oct 2, 2026 by maraf via pat_pool #7457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants