JIT: Improve Wasm PEP calls

[AndyAyersMS]

Summary

• reuse the managed PEP call target's Wasm local for both the final PEP argument and the indirect-call target
• remove the explicit PEP local and use one lowering/codegen path for helper and non-helper calls
• improve the inefficient PEP call sequence reported in [wasm] Performance of GetGCStaticBase could be significantly faster #134450 without attempting to address the other issues discussed there

Example

For System.UInt32.GetHashCode, the null-throw helper path previously stored the PEP to the shadow stack and loaded it twice:

local.get 0
global.get 4326536
i32.load 0 reloc 0x420478
i32.store 0 12
local.get 0
i32.load 0 12
local.get 0
i32.load 0 12
i32.load 0 0
call_indirect 4326584 0

It now tees the PEP into the existing Wasm local and reuses it for the indirect target:

local.get 0
global.get 4326536
i32.load 0 reloc 0x420478
local.tee 3
local.get 3
i32.load 0 0
call_indirect 4326584 0

This method decreases from 116 to 105 bytes.

Code size

Browser CoreLib SuperPMI, 28,985 successful contexts:

	Bytes
Baseline	9,973,415
Diff	9,746,072
Change	-227,343 (-2.2795%)

16,849 methods improve, 9 regress, and 12,127 are unchanged.

Testing

• Checked Browser/Wasm native libraries, CoreCLR runtime, cross-JIT, and CoreLib builds
• full Browser CoreLib SuperPMI replay: 28,985/28,985 contexts succeeded
• Node execution using a newly built, hash-matched Checked runtime, CoreLib, cross-JIT, and Core_Root:
  • deadoponerrorinfunclet_d
  • deadoponerrorinfunclet_r
  • simpledeadehregion_r

Note

This PR description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[AndyAyersMS]

@adamperlin PTAL
fyi @dotnet/wasm-contrib

Code size delta is from SPMI, actual browser corelib size should be similar, but I'll verify.

(looks like it is more savings in bytes, lower percent overall given the other stuff in the file)

[Copilot]

Copilot review overview

🔵 Needs a closer look

Wasm JIT lowering, register allocation, and code generation changes require final human review.

Review effort: Lite
Findings: None

What changed in this PR

Improves WebAssembly PEP call generation by reusing the target local, reducing shadow-stack traffic and code size.

Changes:

• Updates Wasm register allocation for PEP target temporaries.
• Uses a contained ABI placeholder during lowering.
• Reuses the target local for indirect-call emission.

File	Description
src/​coreclr/​jit/​regallocwasm.cpp	Manages PEP target temporary lifetime.
src/​coreclr/​jit/​lowerwasm.cpp	Lowers PEP calls using the original control expression.
src/​coreclr/​jit/​codegenwasm.cpp	Emits the optimized indirect-call sequence.

[dotnet-policy-service]

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The changes affect JIT lowering, register allocation, and code generation and warrant final human review.

Review effort: Lite
Findings: None

[jakobbotsch]

What is the IR here? I'm curious why the straightforward IR representation with a local for the PEP argument and GT_IND on top of that local for the call target doesn't get good codegen.

[AndyAyersMS]

This PR is possibly obsolete given that #134555 is changing over to making direct calls to helpers.

What is the IR here? I'm curious why the straightforward IR representation with a local for the PEP argument and GT_IND on top of that local for the call target doesn't get good codegen.

Yeah it is trying to be clever and use wasm locals for this instead. Let me look at a more conventional solution.

[adamperlin]

This PR is possibly obsolete given that #134555 is changing over to making direct calls to helpers.

This should still have positive diffs for all non-helper managed calls though I would imagine?

[davidwrighton]

@AndyAyersMS, this PR isn't obsolete. It still improves the case when the module we compile ISN'T also containing System.Private.CoreLib, or when the helper is written in C. I'm going to probably try to address the C helper situation, since those should be able to use a function import, but the modular R2R file istuation would be good to fix.

[AndyAyersMS]

Actual problem is that locals introduced post-lower aren't tracked, so aren't considered as register candidates, so for helper calls (added in stack level setter) the PEP local is put on the frame. Seems fixable.

[edit] Though not perhaps fixable without some other similar hackiness. Either I rebuild liveness, adopt a temporary side-car liveness bypass like the one physical promotion uses, teach Wasm RA via some other means that this local is special, or do something like this PR that leverages Wasm multi-use nodes to avoid introducing a local.

[jakobbotsch]

Actual problem is that locals introduced post-lower aren't tracked, so aren't considered as register candidates, so for helper calls (added in stack level setter) the PEP local is put on the frame. Seems fixable.

[edit] Though not perhaps fixable without some other similar hackiness. Either I rebuild liveness, adopt a temporary side-car liveness bypass like the one physical promotion uses, teach Wasm RA via some other means that this local is special, or do something like this PR that leverages Wasm multi-use nodes to avoid introducing a local.

Can we move the existing liveness to run after the stack level setter? I don't think we depend on the current ordering of those.

[AndyAyersMS]

car liveness bypass like the one physical promotion uses, teach Wasm RA via some other means that this local is special, or do something like this PR that leverages Wasm multi-use nodes to avoid introducing a local.

Can we move the existing liveness to run after the stack level setter? I don't think we depend on the current ordering of those

Almost, yes. We introduced a dependence recently; I suppose I can just bring back that bit of code.

runtime/src/coreclr/jit/liveness.cpp

Lines 1305 to 1308 in f3fc53d

	// Now that we create throw helper blocks after lower,
	// we don't need to search for them and set up liveness
	// during lower.
	assert(!m_compiler->fgRngChkThrowAdded);

[AndyAyersMS]

Almost, yes. We introduced a dependence recently; I suppose I can just bring back that bit of code.

That doesn't quite work since we introduce a new local but not new var death flags so liveness and codegen live vars now seemingly diverge. Looking into fixing that now.

[AndyAyersMS]

Ok, got the deferred liveness to work.

Had to recompute live in/last use for throw helpers, verified that it does not lead to new global liveness.

Similar size impact as before (a bit less as the pep locals each get their own wasm local, which can increase encoding size). We could fix this with a "real RA" that assigns different lcl vars to the same wasm local.

Some small diffs on native targets, as zero weight blocks may be in slightly different orders now.

@adamperlin @jakobbotsch ptal

[Copilot]

Copilot review overview

🔵 Needs a closer look

Finalization ordering must be corrected or outgoing argument space recomputed.

Review effort: Lite
Findings: None

[AndyAyersMS]

Size impact of this version vs previous -- we lost about 12K bytes.

[AndyAyersMS]

spmi diffs

[jakobbotsch]

LGTM.

I wonder where the TP regressions are coming from... Is it just the throw helper liveness?

[AndyAyersMS]

LGTM.

I wonder where the TP regressions are coming from... Is it just the throw helper liveness?

Two things:

• minopts: unnecessary ref count computation
• fullopts: unnecessary DFS invalidation/recompute

Both fixed, TP impact should now be minimal

[AndyAyersMS]

@jakobbotsch re-approve?
(or @adamperlin)

[Copilot]

Copilot review overview

🔵 Needs a closer look

The JIT phase-ordering and liveness changes require final human review.

Review effort: Lite
Findings: None

[AndyAyersMS]

From the updated diffs