[RyuJIT Wasm] Implement i64/f32/f64 loads and adds

[kg]

My first JIT PR so I'm keeping it simple.

[dotnet-policy-service]

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

[kg]

@dotnet/jit-contrib

[Copilot]

Pull request overview

This PR implements basic arithmetic operations (loads and adds) for i64, f32, and f64 types in the RyuJIT WebAssembly backend. The changes enable the JIT compiler to generate WebAssembly instructions for these fundamental operations across 64-bit integers, 32-bit floats, and 64-bit floats.

• Implements register validation functions for integer and float registers based on WASM value types
• Adds WASM instruction definitions for i64/f32/f64 load and add operations
• Updates code generation to handle typed binary operations using a new helper function

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/coreclr/jit/targetwasm.h	Replaces NYI stubs with actual implementations of register validation functions that check WASM value types (I32, I64, F32, F64)
src/coreclr/jit/instrswasm.h	Adds instruction definitions for i64_load, f32_load, f64_load, i64_add, f32_add, and f64_add with their corresponding opcodes
src/coreclr/jit/instr.cpp	Extends ins_Load to support TYP_LONG, TYP_FLOAT, and TYP_DOUBLE by mapping them to appropriate WASM load instructions
src/coreclr/jit/codegenwasm.cpp	Introduces OperAndType helper to combine operation and type into a switch value, refactors genCodeForBinary to handle GT_ADD for INT, LONG, FLOAT, and DOUBLE types
src/coreclr/jit/lower.cpp	Adds workaround for NumSegments being 0 on WASM to prevent crashes when creating GT_PUTARG_STK nodes

[jakobbotsch]

When does this happen?

WasmClassifier::Classify always returns a single register segment so this seems odd.
Also with the assert above this doesn't make sense.

[kg]

While debugging crossgen we get past the assert into this code and we get an AV because Segment(0) is an invalid pointer.

[jakobbotsch]

The fix should be to make sure there is some reasonable ABI info there for WASM. But given the implementation of WasmClassifier it is odd that there isn't, so we should figure out why.

The assert above will keep firing for people running the JIT in debug/checked so some other fix is needed.

[kg]

I'll look into it further, but with how little I know about the JIT I don't expect to get very far. Should I pair with someone?

[jakobbotsch]

That sounds like a good idea. A good place to start would be by obtaining the JITDUMP for the function where the AV occurs. The example at https://github.com/dotnet/runtime/blob/main/docs/workflow/debugging/coreclr/debugging-aot-compilers.md#example-of-debugging-a-test-application-in-crossgen2 might help with that.
Also feel free to hit me up internally.

[SingleAccretion]

Also, is this needed for the current work? This code is part of support for calls.

[kg]

It gets hit when I debug crossgen2. It works fine outside of the debugger.

[SingleAccretion]

Suggested change

	constexpr uint64_t PackOperAndType(genTreeOps oper, var_types type)
	{
	return ((uint64_t)oper) | ((uint64_t)type << 32);
	}
	static constexpr uint64_t PackOperAndType(genTreeOps oper, var_types type)
	{
	return ((uint64_t)oper) | ((uint64_t)type << 32);
	}

You could make this more likely to produce a dense series (and fit into an int32) by shifting oper by ConstLog2<GT_COUNT>::value + 1 and ORing the type, since type is the part with the dense lower bits. That said, I don't know how much that would help; the series will still be sparse. It may help on RISC architectures that don't have instructions for loading large constants, I suppose.

[SingleAccretion]

The way this function is currently structured is leaving performance on the table. It would be best to remove this and lower part that is dealing with FP and integers separately, and have one switch for both at the beginning.

(Under a WASM ifdef, of course)

[SingleAccretion]

This function lacks the context to determine whether it is an arg reg or not. What stack is this being called from? The correct fix may be to put the caller under HAS_FIXED_REGISTER_SET. Or propagate the context (on the number of args) here.

[kg]

I just decided to fill out all of them while I was here. I can revert it.

[SingleAccretion]

Yes, that would be best.

The genIsValidInt/FloatReg have clear semantics under !HAS_FIXED_REGISTER_SET. These Arg functions don't, so the best thing would be to make them unreachable at some point, unless we find it necessary to properly implement them.

Separately, genIsValidIntReg/etc should be moved to registeropswasm.cpp (you will need to add prototypes for them to registeropswasm.h, like with genIsValidReg).