[adamperlin]

Resolves #134465; We can re-order an array index computation to allow the array data offset to fold into a load/store using our existing containment logic, subject to its constraints (in particular, the address must be TYP_REF or TYP_BYREF). It turns out that this kind of reordering is actually already done for risc v.
Diffs

[azure-pipelines]

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.

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🔵 Needs a closer look

The JIT transformation appears sound, but its claimed Wasm code-size improvement lacks direct validation in the PR.

Review effort: Balanced
Findings: None

What changed in this PR

Optimizes Wasm array access code generation by reshaping address arithmetic so the array data offset can fold into the memory instruction.

Changes:

• Uses (array + scaledIndex) + dataOffset for eligible non-struct arrays.
• Restricts reshaping to GC-typed array references.

File	Description
src/​coreclr/​jit/​morph.cpp	Adds Wasm-specific array address grouping for memarg containment.

[adamperlin]

cc @dotnet/wasm-contrib

[EgorBo]

in particular, the address must be TYP_REF or TYP_BYREF

I am not sure I understand why we check this. I assume arr_ref can also be a TYP_I_IMPL (e.g. local alloc) but is there a reason to limit on that?

[adamperlin]

@EgorBo the only reason to limit is because the offset folding logic we have in place only supports TYP_REF/TYP_BYREF (See the comment here:

runtime/src/coreclr/jit/lowerwasm.cpp

Line 174 in 523a36f

// The memarg offset is added in infinite precision, while "i32.add" wraps, so the two only differ when

)
The issue is with different overflow behavior between a non-contained and contained address computation on Wasm. So, I think we could allow TYP_I_IMPL here, it just wouldn't benefit from the folding based on the rules we have in place.

[EgorBo]

@EgorBo the only reason to limit is because the offset folding logic we have in place only supports TYP_REF/TYP_BYREF (See the comment here:

runtime/src/coreclr/jit/lowerwasm.cpp

Line 174 in 523a36f

// The memarg offset is added in infinite precision, while "i32.add" wraps, so the two only differ when

)
The issue is with different overflow behavior between a non-contained and contained address computation on Wasm. So, I think we could allow TYP_I_IMPL here, it just wouldn't benefit from the folding based on the rules we have in place.

@adamperlin my understanding that on wasm the addressing mode has to never overflow and we can rely on live GC pointers + offset to never overflow - which is legal. But TYP_BYREF itself has no guarantee - it can be just some arbitrary native address and +cns may overflow it. So technically only TYP_REF is legal. But I think in this case it's fine, the arrayObj is either stack allocated or real gc ref.

[adamperlin]

@EgorBo got it. Yes that's the correct, on Wasm the addressing mode will trap on overflow, but if we compute the address before the load with i32.add $base $offset; i32.load, we will have wraparound on overflow. If TYP_BYREF could overflow with +cns, then maybe the right thing to do is to limit the folding like you're suggesting. I can make that fix.

[adamperlin]

/ba-g unrelated infra failures