[RyuJIT] Make the mis-sized struct return logic only apply to return values that are structs

[kg]

This addresses an assert in wasm codegen with a release SPC, i.e.:

--targetos:browser
--targetarch:wasm
--obj-format=wasm
--parallelism:1
--print-repro-instructions
--codegenopt:JitWasmNyiToR2RUnsupported=1
--codegenopt:JitDump=System.Nullable`1[System.Reflection.Metadata.TypeNameParseOptions]:op_Explicit
-r:"Z:\runtime\artifacts\tests\coreclr\browser.wasm.Release\Tests\Core_Root\*.dll"
"Z:\runtime\artifacts\tests\coreclr\browser.wasm.Release\Tests\Core_Root\System.Private.CoreLib.dll"
--out:"Z:\runtime\artifacts\spc.wasm"
--singlemethodtypename:"System.Nullable`1[[System.Reflection.Metadata.TypeNameParseOptions]]"
--singlemethodname:"op_Explicit"
--singlemethodindex:1

Would produce:

Single method repro args:--singlemethodtypename "System.Nullable`1[[System.Reflection.Metadata.TypeNameParseOptions]]" --singlemethodname "op_Explicit" --singlemethodindex 1
N004 (  7,  7) [000020] n---G+-----                         *  IND       ubyte  <l:$141, c:$181>
N003 (  3,  4) [000019] -----+-----                         \--*  ADD       byref  $200
N001 (  1,  1) [000012] -----+-----                            +--*  LCL_VAR   byref  V01 arg0         u:1 (last use) $c0
N002 (  1,  2) [000018] -----+-----                            \--*  CNS_INT   int    1 $41
Z:\runtime\src\coreclr\jit\gentree.cpp:16961
Assertion failed '!"Incompatible types for gtNewTempStore"' in 'System.Nullable`1[System.Reflection.Metadata.TypeNameParseOptions]:op_Explicit(System.Nullable`1[System.Reflection.Metadata.TypeNameParseOptions]):System.Reflection.Metadata.TypeNameParseOptions' during 'Lowering nodeinfo' (IL size 8; hash 0xbc4e1b69; FullOpts)

The problem is that we are creating a temporary local of struct type using the retTypeClass, but the type of the return value was erased, so we try to turn a GT_IND of type TYP_UBYTE into a temp of TYP_STRUCT which can't possibly work the way ReplaceWithLclVar is constructed.

EDIT:
In this scenario what we need to do is zero-extend the mis-sized return value (a byte or short, most likely) to match the native return type's size. So I wrap the return value in a CAST.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Fixes a RyuJIT lowering assertion encountered in WASM codegen when a struct return’s value node has an erased (non-struct) type, which caused an invalid spill-to-struct-temp transformation.

Changes:

• Guard the mis-sized struct return “spill to temp local” workaround so it only runs when the return value node is actually TYP_STRUCT.
• Avoid invoking ReplaceWithLclVar to store a non-struct-typed GT_IND into a struct-typed temp local.

[Copilot]

The added comment says to ensure the retval is actually a struct, but in this code path the method return can be a struct even when retVal has an erased primitive type (which is the scenario being fixed). Consider rewording to clarify you mean the tree is TYP_STRUCT (i.e., struct-typed indir) to avoid confusion for future readers.

Suggested change

	// Ensure that the retval is actually a struct - otherwise the ReplaceWithLclVar below
	// will fail due to assigning a non-struct to a struct local
	// Ensure that the return-value tree is TYP_STRUCT; otherwise the ReplaceWithLclVar
	// below will fail due to assigning a non-struct tree to a struct local.

[kg]

I misread something and this is incorrect. Will keep working on it.

[Copilot]

Pull request overview

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

[Copilot]

The new widening path builds a GT_CAST with castType = varTypeToUnsigned(nativeReturnType). That makes the cast’s semantic target type UINT/U_IMPL (same width as nativeReturnType), so it no longer encodes the required zero-extension of the mismatched load size (e.g., UBYTE/USHORT) and can become a no-op cast on some targets. For correct/portable zero-extension without widening the memory read, the cast should keep the destination node type as the actual native return type (e.g., genActualType(nativeReturnType) / ret->TypeGet()), but set CastToType to an unsigned small type based on the indirection’s type (e.g., varTypeToUnsigned(retVal->TypeGet())), so the cast represents a zero-extending small→int/native-int conversion.

Suggested change

	// Instead, wrap the indirection in a cast to zero extend it.
	GenTreeCast* cast =
	m_compiler->gtNewCastNode(nativeReturnType, retVal, true, varTypeToUnsigned(nativeReturnType));
	// Instead, wrap the indirection in a cast to zero extend it from the actual load size.
	GenTreeCast* cast =
	m_compiler->gtNewCastNode(nativeReturnType, retVal, true, varTypeToUnsigned(retVal->TypeGet()));

[kg]

Did I misunderstand the comments around cast nodes @AndyAyersMS ? What I'm trying to do here is 'cast to the unsigned version of the return type' and then have the cast result masquerade as the actual return type, in order to get a zero-extend.

[AndyAyersMS]

Cast semantics are a frequent source of confusion, because logically they accept and return stack types but they can operate on non-stack types.

The last type arg here should be a small type, the type of the indir.

You can assert cast->IsZeroExtending() after you create it to make sure it agrees with what you expect.