[wasm] Wasm C ABI correctness for "singleton" structs in the mono AOT compiler and interpreter #94895
Comments
dotnet-issue-labeler
bot
added
the
needs-area-label
kg
added
arch-wasm
ghost
added
the
|
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsThe 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:
The current PInvoke generator doesn't support any of this (I'm working on fixing it in #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 * When I say 'work' above I mean 'compile'; the compiled code still does not work right in either the interpreter or AOT: In the interpreter, the code will run but either gets garbage or 0 instead of the struct instance: 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
|
kg
added
area-Codegen-AOT-mono
and removed
untriaged
|
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: For a pinvoke accepting a struct and returning a float: For a pinvoke accepting a struct and returning a struct: 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. |
|
Tagging subscribers to this area: @BrzVlad, @kotlarmilos |
|
I believe a lot of this is fixed but I don't think I added enough coverage to guarantee that it's all fixed. |
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:
The current PInvoke generator doesn't support any of this (I'm working on fixing it in #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_vtypeinmini-wasm.cwe 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:* When I say 'work' above I mean 'compile'; the compiled code still does not work right in either the interpreter or AOT:
In the interpreter, the code will run but either gets garbage or 0 instead of the struct instance:
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:
cc @vargaz
The text was updated successfully, but these errors were encountered: