[wasm] Wasm C ABI correctness for "singleton" structs in the mono AOT compiler and interpreter

[kg]

The wasm ABI https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md#function-signatures appears to imply that the following cases will all become scalars and be passed by-value instead of by-reference:

• A struct only containing one F32 (A)
• A struct only containing one F64 (B)
• A struct only containing one I32 (C)
• A struct only containing one I64 (D)

The current PInvoke generator doesn't support any of this (I'm working on fixing it in https://github.com/dotnet/runtime/pull/94446) so you may need to use my PR in order to test.

At present we only seem to implement case A & case C reliably. By updating mini_wasm_is_scalar_vtype in mini-wasm.c we can make case B work* in the AOT compiler but if we also implement case D by allowing I8/U8 types through, we get assertion failures in the AOT compiler:

Assertion at /home/kate/Projects/dotnet-runtime-wasm/src/mono/mono/mini/mini-llvm.c:6052, condition `addresses [ins->sreg1]' not met

* When I say 'work' above I mean 'compile'; the compiled code still does not work right in either the interpreter or AOT:

[wasm test-browser] info: &arg=4ff270 (ulonglong)arg=40091eb851eb851f arg.value.value=3.140000
[wasm test-browser] info: resF=3.14
[wasm test-browser] fail: MONO_WASM: RuntimeError: null function or function signature mismatch
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[96]:0x18168
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[89]:0x17cdb
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[12890]:0x1a2a2a8
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18247]:0x2ff814a
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18500]:0x3017cb7
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18169]:0x2ff5165
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18159]:0x2fe75f3
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18202]:0x2ff6272
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[22479]:0x30e192a
[wasm test-browser]           at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[21430]:0x30ad1ae

In the interpreter, the code will run but either gets garbage or 0 instead of the struct instance:

[wasm test-browser] info: &arg=5d9c68 (ulonglong)arg=40091eb851eb851f arg.value.value=3.140000
[wasm test-browser] info: resF=3.14
[wasm test-browser] info: &arg=5d9988 (ulonglong)arg=0 arg.value.value=0.000000
[wasm test-browser] info: res=0

I think this is because the interpreter has its own logic for identifying scalar vtypes, but when I attempted to fix it that caused all sorts of problems.

Sample code I used for testing:

#include <stdio.h>

typedef struct {
    float value;
} TRes;

TRes accept_double_struct_and_return_float_struct (
    struct { struct { double value; } value; } arg
) {
    printf (
        "&arg=%x (ulonglong)arg=%llx arg.value.value=%lf\n",
        (unsigned int)&arg, *(unsigned long long*)&arg, (double)arg.value.value
    );
    TRes result = { arg.value.value };
    return result;
}

using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

public struct SingleFloatStruct {
    public float Value;
}
public struct SingleDoubleStruct {
    public struct Nested1 {
        // This field is private on purpose to ensure we treat visibility correctly
        double Value;
    }
    public Nested1 Value;
}

public class Test
{
    public static unsafe int Main(string[] argv)
    {
        var resF = direct(3.14);
        Console.WriteLine(""resF="" + resF);

        SingleDoubleStruct sds = default;
        Unsafe.As<SingleDoubleStruct, double>(ref sds) = 3.14; // I tested using pointers instead of Unsafe.As and that also doesn't work
        var res = indirect(sds);
        Console.WriteLine(""res="" + res.Value);
        return (int)res.Value;
    }

    [DllImport(""wasm-abi"", EntryPoint=""accept_double_struct_and_return_float_struct"")]
    public static extern SingleFloatStruct indirect(SingleDoubleStruct arg);

    [DllImport(""wasm-abi"", EntryPoint=""accept_double_struct_and_return_float_struct"")]
    public static extern float direct(double arg);
}

cc @vargaz

[ghost]

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

Issue Details

---

The wasm ABI https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md#function-signatures appears to imply that the following cases will all become scalars and be passed by-value instead of by-reference: * A struct only containing one F32 (A) * A struct only containing one F64 (B) * A struct only containing one I32 (C) * A struct only containing one I64 (D) The current PInvoke generator doesn't support any of this (I'm working on fixing it in https://github.com/dotnet/runtime/pull/94446) so you may need to use my PR in order to test. At present we only seem to implement case A & case C reliably. By updating `mini_wasm_is_scalar_vtype` in `mini-wasm.c` we can make case B work\* in the AOT compiler but if we also implement case D by allowing I8/U8 types through, we get assertion failures in the AOT compiler: ``` Assertion at /home/kate/Projects/dotnet-runtime-wasm/src/mono/mono/mini/mini-llvm.c:6052, condition `addresses [ins->sreg1]' not met ``` \* When I say 'work' above I mean 'compile'; the compiled code still does not work right in either the interpreter or AOT: ``` [wasm test-browser] info: &arg=4ff270 (ulonglong)arg=40091eb851eb851f arg.value.value=3.140000 [wasm test-browser] info: resF=3.14 [wasm test-browser] fail: MONO_WASM: RuntimeError: null function or function signature mismatch [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[96]:0x18168 [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[89]:0x17cdb [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[12890]:0x1a2a2a8 [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18247]:0x2ff814a [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18500]:0x3017cb7 [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18169]:0x2ff5165 [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18159]:0x2fe75f3 [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[18202]:0x2ff6272 [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[22479]:0x30e192a [wasm test-browser] at http://127.0.0.1:38685/_framework/dotnet.native.wasm:wasm-function[21430]:0x30ad1ae ``` In the interpreter, the code will run but either gets garbage or 0 instead of the struct instance: ``` [wasm test-browser] info: &arg=5d9c68 (ulonglong)arg=40091eb851eb851f arg.value.value=3.140000 [wasm test-browser] info: resF=3.14 [wasm test-browser] info: &arg=5d9988 (ulonglong)arg=0 arg.value.value=0.000000 [wasm test-browser] info: res=0 ``` I think this is because the interpreter has its own logic for identifying scalar vtypes, but when I attempted to fix it that caused all sorts of problems. Sample code I used for testing: ```c #include typedef struct { float value; } TRes; TRes accept_double_struct_and_return_float_struct ( struct { struct { double value; } value; } arg ) { printf ( "&arg=%x (ulonglong)arg=%llx arg.value.value=%lf\n", (unsigned int)&arg, *(unsigned long long*)&arg, (double)arg.value.value ); TRes result = { arg.value.value }; return result; } ``` ```csharp using System; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; public struct SingleFloatStruct { public float Value; } public struct SingleDoubleStruct { public struct Nested1 { // This field is private on purpose to ensure we treat visibility correctly double Value; } public Nested1 Value; } public class Test { public static unsafe int Main(string[] argv) { var resF = direct(3.14); Console.WriteLine(""resF="" + resF); SingleDoubleStruct sds = default; Unsafe.As(ref sds) = 3.14; // I tested using pointers instead of Unsafe.As and that also doesn't work var res = indirect(sds); Console.WriteLine(""res="" + res.Value); return (int)res.Value; } [DllImport(""wasm-abi"", EntryPoint=""accept_double_struct_and_return_float_struct"")] public static extern SingleFloatStruct indirect(SingleDoubleStruct arg); [DllImport(""wasm-abi"", EntryPoint=""accept_double_struct_and_return_float_struct"")] public static extern float direct(double arg); } ``` cc @vargaz

Author:	kg
Assignees:	-
Labels:	`arch-wasm`
Milestone:	-

[kg]

As of https://github.com/dotnet/runtime/pull/94446/commits/3a278eeb7c046ffe12ffbfeb6f955f382f51667a in #94446 basic cases now work in the interpreter, I had to fix some assumptions about scalarized structs (i.e. they're always int32) and also update the return value marshaling to handle scalarized return values. Will investigate AOT more.

[kg]

The AOT wrapper appears to invoke using the wrong indirect signature when structs are involved. For a pinvoke with the signature 'float (double)', it's correct:

75336 117100    call_indirect 113 tables[0] s(d)

For a pinvoke accepting a struct and returning a float:

75201 11D70100  call_indirect 215 tables[0] s(l)

For a pinvoke accepting a struct and returning a struct:

75067 115300    call_indirect 83 tables[0] i(l)

It looks like the problem is it's picking an appropriately-sized int to contain the struct, but in this case it needs to pick an appropriately-sized float instead.

[dotnet-policy-service[bot]]

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

[kg]

I believe a lot of this is fixed but I don't think I added enough coverage to guarantee that it's all fixed.