[Wasm RyuJit] More crossgen assert fixes

[AndyAyersMS]

• use gtOverflowEx instead of gtOverflow
• not all block stores need null checks
• more cases where we don't have putarg stacks
• add lowering was bypassing Wasm overflow checking
• tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
• storeind nullchecks

[AndyAyersMS]

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

[Copilot]

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

• Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
• Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
• Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
• Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/lowerwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in LowerBinaryArithmetic for correctness when called with non-overflow-capable operators (AND/OR/XOR)
src/coreclr/jit/lower.cpp	Fix MovePutArgUpToCall/MovePutArgNodesUpToCall asserts for WASM (no PutArg nodes); add new else branch for plain arg nodes; call LowerBinaryArithmetic from LowerAdd on WASM to fix missing overflow setup for ADD
src/coreclr/jit/codegenwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in genCodeForBinary; gate null checks in genCodeForCpObj behind doNullChecks for correctness when copies are known safe

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[SingleAccretion]

LGTM modulo comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

• In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
if (node->OperIs(GT_ADD))
{
LowerBinaryArithmetic(node);
}
#endif
if (node->OperIs(GT_ADD))
{
ContainCheckBinary(node);
}

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[AndyAyersMS]

• use gtOverflowEx instead of gtOverflow
• not all block stores need null checks
• more cases where we don't have putarg stacks
• add lowering was bypassing Wasm overflow checking
• tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
• storeind nullchecks

[AndyAyersMS]

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

[Copilot]

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

• Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
• Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
• Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
• Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/lowerwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in LowerBinaryArithmetic for correctness when called with non-overflow-capable operators (AND/OR/XOR)
src/coreclr/jit/lower.cpp	Fix MovePutArgUpToCall/MovePutArgNodesUpToCall asserts for WASM (no PutArg nodes); add new else branch for plain arg nodes; call LowerBinaryArithmetic from LowerAdd on WASM to fix missing overflow setup for ADD
src/coreclr/jit/codegenwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in genCodeForBinary; gate null checks in genCodeForCpObj behind doNullChecks for correctness when copies are known safe

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[SingleAccretion]

LGTM modulo comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

• In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
if (node->OperIs(GT_ADD))
{
LowerBinaryArithmetic(node);
}
#endif
if (node->OperIs(GT_ADD))
{
ContainCheckBinary(node);
}

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[AndyAyersMS]

• use gtOverflowEx instead of gtOverflow
• not all block stores need null checks
• more cases where we don't have putarg stacks
• add lowering was bypassing Wasm overflow checking
• tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
• storeind nullchecks

[AndyAyersMS]

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

[Copilot]

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

• Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
• Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
• Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
• Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/lowerwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in LowerBinaryArithmetic for correctness when called with non-overflow-capable operators (AND/OR/XOR)
src/coreclr/jit/lower.cpp	Fix MovePutArgUpToCall/MovePutArgNodesUpToCall asserts for WASM (no PutArg nodes); add new else branch for plain arg nodes; call LowerBinaryArithmetic from LowerAdd on WASM to fix missing overflow setup for ADD
src/coreclr/jit/codegenwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in genCodeForBinary; gate null checks in genCodeForCpObj behind doNullChecks for correctness when copies are known safe

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[SingleAccretion]

LGTM modulo comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

• In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
if (node->OperIs(GT_ADD))
{
LowerBinaryArithmetic(node);
}
#endif
if (node->OperIs(GT_ADD))
{
ContainCheckBinary(node);
}

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[AndyAyersMS]

• use gtOverflowEx instead of gtOverflow
• not all block stores need null checks
• more cases where we don't have putarg stacks
• add lowering was bypassing Wasm overflow checking
• tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
• storeind nullchecks

[AndyAyersMS]

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

[Copilot]

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

• Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
• Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
• Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
• Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/lowerwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in LowerBinaryArithmetic for correctness when called with non-overflow-capable operators (AND/OR/XOR)
src/coreclr/jit/lower.cpp	Fix MovePutArgUpToCall/MovePutArgNodesUpToCall asserts for WASM (no PutArg nodes); add new else branch for plain arg nodes; call LowerBinaryArithmetic from LowerAdd on WASM to fix missing overflow setup for ADD
src/coreclr/jit/codegenwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in genCodeForBinary; gate null checks in genCodeForCpObj behind doNullChecks for correctness when copies are known safe

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[SingleAccretion]

LGTM modulo comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

• In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
if (node->OperIs(GT_ADD))
{
LowerBinaryArithmetic(node);
}
#endif
if (node->OperIs(GT_ADD))
{
ContainCheckBinary(node);
}

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[AndyAyersMS]

• use gtOverflowEx instead of gtOverflow
• not all block stores need null checks
• more cases where we don't have putarg stacks
• add lowering was bypassing Wasm overflow checking
• tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
• storeind nullchecks

[AndyAyersMS]

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

[Copilot]

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

• Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
• Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
• Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
• Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/lowerwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in LowerBinaryArithmetic for correctness when called with non-overflow-capable operators (AND/OR/XOR)
src/coreclr/jit/lower.cpp	Fix MovePutArgUpToCall/MovePutArgNodesUpToCall asserts for WASM (no PutArg nodes); add new else branch for plain arg nodes; call LowerBinaryArithmetic from LowerAdd on WASM to fix missing overflow setup for ADD
src/coreclr/jit/codegenwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in genCodeForBinary; gate null checks in genCodeForCpObj behind doNullChecks for correctness when copies are known safe

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[SingleAccretion]

LGTM modulo comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

• In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
if (node->OperIs(GT_ADD))
{
LowerBinaryArithmetic(node);
}
#endif
if (node->OperIs(GT_ADD))
{
ContainCheckBinary(node);
}

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[AndyAyersMS]

• use gtOverflowEx instead of gtOverflow
• not all block stores need null checks
• more cases where we don't have putarg stacks
• add lowering was bypassing Wasm overflow checking
• tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
• storeind nullchecks

[AndyAyersMS]

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

[Copilot]

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

• Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
• Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
• Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
• Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/lowerwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in LowerBinaryArithmetic for correctness when called with non-overflow-capable operators (AND/OR/XOR)
src/coreclr/jit/lower.cpp	Fix MovePutArgUpToCall/MovePutArgNodesUpToCall asserts for WASM (no PutArg nodes); add new else branch for plain arg nodes; call LowerBinaryArithmetic from LowerAdd on WASM to fix missing overflow setup for ADD
src/coreclr/jit/codegenwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in genCodeForBinary; gate null checks in genCodeForCpObj behind doNullChecks for correctness when copies are known safe

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[SingleAccretion]

LGTM modulo comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

• In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
if (node->OperIs(GT_ADD))
{
LowerBinaryArithmetic(node);
}
#endif
if (node->OperIs(GT_ADD))
{
ContainCheckBinary(node);
}

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[AndyAyersMS]

• use gtOverflowEx instead of gtOverflow
• not all block stores need null checks
• more cases where we don't have putarg stacks
• add lowering was bypassing Wasm overflow checking
• tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
• storeind nullchecks

[AndyAyersMS]

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

[Copilot]

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

• Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
• Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
• Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
• Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/lowerwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in LowerBinaryArithmetic for correctness when called with non-overflow-capable operators (AND/OR/XOR)
src/coreclr/jit/lower.cpp	Fix MovePutArgUpToCall/MovePutArgNodesUpToCall asserts for WASM (no PutArg nodes); add new else branch for plain arg nodes; call LowerBinaryArithmetic from LowerAdd on WASM to fix missing overflow setup for ADD
src/coreclr/jit/codegenwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in genCodeForBinary; gate null checks in genCodeForCpObj behind doNullChecks for correctness when copies are known safe

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[SingleAccretion]

LGTM modulo comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

• In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
if (node->OperIs(GT_ADD))
{
LowerBinaryArithmetic(node);
}
#endif
if (node->OperIs(GT_ADD))
{
ContainCheckBinary(node);
}

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[AndyAyersMS]

• use gtOverflowEx instead of gtOverflow
• not all block stores need null checks
• more cases where we don't have putarg stacks
• add lowering was bypassing Wasm overflow checking
• tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
• storeind nullchecks

[AndyAyersMS]

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

[Copilot]

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

• Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
• Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
• Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
• Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/lowerwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in LowerBinaryArithmetic for correctness when called with non-overflow-capable operators (AND/OR/XOR)
src/coreclr/jit/lower.cpp	Fix MovePutArgUpToCall/MovePutArgNodesUpToCall asserts for WASM (no PutArg nodes); add new else branch for plain arg nodes; call LowerBinaryArithmetic from LowerAdd on WASM to fix missing overflow setup for ADD
src/coreclr/jit/codegenwasm.cpp	Use gtOverflowEx() instead of gtOverflow() in genCodeForBinary; gate null checks in genCodeForCpObj behind doNullChecks for correctness when copies are known safe

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[SingleAccretion]

LGTM modulo comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

• In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
if (node->OperIs(GT_ADD))
{
LowerBinaryArithmetic(node);
}
#endif
if (node->OperIs(GT_ADD))
{
ContainCheckBinary(node);
}

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.