From 915ee6db9cb871bb2ef32698015bca0f2c895ce8 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Fri, 3 Dec 2021 14:38:03 -0500 Subject: [PATCH] [mono][wasm] Fix the passing/returning of small vtypes. (#62299) According to the ABI, they need to be returned by value. --- src/mono/mono/mini/mini-llvm.c | 36 ++++++++++++++++++- src/mono/mono/mini/mini-wasm.c | 64 ++++++++++++++++++++++++++++++---- src/mono/mono/mini/mini.h | 8 ++++- 3 files changed, 100 insertions(+), 8 deletions(-) diff --git a/src/mono/mono/mini/mini-llvm.c b/src/mono/mono/mini/mini-llvm.c index 0d19ee09a9bb4e..5830c336da380a 100644 --- a/src/mono/mono/mini/mini-llvm.c +++ b/src/mono/mono/mini/mini-llvm.c @@ -1568,6 +1568,10 @@ sig_to_llvm_sig_full (EmitContext *ctx, MonoMethodSignature *sig, LLVMCallInfo * vretaddr = TRUE; ret_type = LLVMVoidType (); break; + case LLVMArgWasmVtypeAsScalar: + g_assert (cinfo->ret.esize); + ret_type = LLVMIntType (cinfo->ret.esize * 8); + break; default: break; } @@ -1685,6 +1689,10 @@ sig_to_llvm_sig_full (EmitContext *ctx, MonoMethodSignature *sig, LLVMCallInfo * case LLVMArgVtypeAsScalar: g_assert_not_reached (); break; + case LLVMArgWasmVtypeAsScalar: + g_assert (ainfo->esize); + param_types [pindex ++] = LLVMIntType (ainfo->esize * 8); + break; case LLVMArgGsharedvtFixed: case LLVMArgGsharedvtFixedVtype: param_types [pindex ++] = LLVMPointerType (type_to_llvm_arg_type (ctx, ainfo->type), 0); @@ -3825,6 +3833,7 @@ emit_entry_bb (EmitContext *ctx, LLVMBuilderRef builder) char *name; pindex = ainfo->pindex; + LLVMValueRef arg = LLVMGetParam (ctx->lmethod, pindex); switch (ainfo->storage) { case LLVMArgVtypeInReg: @@ -3883,6 +3892,16 @@ emit_entry_bb (EmitContext *ctx, LLVMBuilderRef builder) case LLVMArgVtypeAsScalar: g_assert_not_reached (); break; + case LLVMArgWasmVtypeAsScalar: { + MonoType *t = mini_get_underlying_type (ainfo->type); + + /* The argument is received as a scalar */ + ctx->addresses [reg] = build_alloca (ctx, t); + + LLVMValueRef dest = convert (ctx, ctx->addresses [reg], LLVMPointerType (LLVMIntType (ainfo->esize * 8), 0)); + LLVMBuildStore (ctx->builder, arg, dest); + break; + } case LLVMArgGsharedvtFixed: { /* These are non-gsharedvt arguments passed by ref, the rest of the IR treats them as scalars */ LLVMValueRef arg = LLVMGetParam (ctx->lmethod, pindex); @@ -4416,6 +4435,10 @@ process_call (EmitContext *ctx, MonoBasicBlock *bb, LLVMBuilderRef *builder_ref, case LLVMArgVtypeAsScalar: g_assert_not_reached (); break; + case LLVMArgWasmVtypeAsScalar: + g_assert (addresses [reg]); + args [pindex] = LLVMBuildLoad (ctx->builder, convert (ctx, addresses [reg], LLVMPointerType (LLVMIntType (ainfo->esize * 8), 0)), ""); + break; case LLVMArgGsharedvtFixed: case LLVMArgGsharedvtFixedVtype: g_assert (addresses [reg]); @@ -4565,6 +4588,11 @@ process_call (EmitContext *ctx, MonoBasicBlock *bb, LLVMBuilderRef *builder_ref, case LLVMArgGsharedvtFixedVtype: values [ins->dreg] = LLVMBuildLoad (builder, convert_full (ctx, addresses [call->inst.dreg], LLVMPointerType (type_to_llvm_type (ctx, sig->ret), 0), FALSE), ""); break; + case LLVMArgWasmVtypeAsScalar: + if (!addresses [call->inst.dreg]) + addresses [call->inst.dreg] = build_alloca (ctx, sig->ret); + LLVMBuildStore (builder, lcall, convert_full (ctx, addresses [call->inst.dreg], LLVMPointerType (LLVMTypeOf (lcall), 0), FALSE)); + break; default: if (sig->ret->type != MONO_TYPE_VOID) /* If the method returns an unsigned value, need to zext it */ @@ -5691,7 +5719,8 @@ process_bb (EmitContext *ctx, MonoBasicBlock *bb) switch (linfo->ret.storage) { case LLVMArgNormal: case LLVMArgVtypeInReg: - case LLVMArgVtypeAsScalar: { + case LLVMArgVtypeAsScalar: + case LLVMArgWasmVtypeAsScalar: { LLVMTypeRef ret_type = LLVMGetReturnType (LLVMGetElementType (LLVMTypeOf (method))); LLVMValueRef retval = LLVMGetUndef (ret_type); gboolean src_in_reg = FALSE; @@ -5748,6 +5777,10 @@ process_bb (EmitContext *ctx, MonoBasicBlock *bb) retval = LLVMBuildLoad (builder, LLVMBuildBitCast (builder, addresses [ins->sreg1], LLVMPointerType (ret_type, 0), ""), ""); } break; + case LLVMArgWasmVtypeAsScalar: + g_assert (addresses [ins->sreg1]); + retval = LLVMBuildLoad (builder, LLVMBuildBitCast (builder, addresses [ins->sreg1], LLVMPointerType (ret_type, 0), ""), ""); + break; } LLVMBuildRet (builder, retval); break; @@ -12163,6 +12196,7 @@ mono_llvm_emit_call (MonoCompile *cfg, MonoCallInst *call) case LLVMArgGsharedvtVariable: case LLVMArgGsharedvtFixed: case LLVMArgGsharedvtFixedVtype: + case LLVMArgWasmVtypeAsScalar: MONO_INST_NEW (cfg, ins, OP_LLVM_OUTARG_VT); ins->dreg = mono_alloc_ireg (cfg); ins->sreg1 = in->dreg; diff --git a/src/mono/mono/mini/mini-wasm.c b/src/mono/mono/mini/mini-wasm.c index 45910e1c2b002f..7e2c7bb1047dcd 100644 --- a/src/mono/mono/mini/mini-wasm.c +++ b/src/mono/mono/mini/mini-wasm.c @@ -23,11 +23,13 @@ typedef enum { ArgValuetypeAddrOnStack, ArgGsharedVTOnStack, ArgValuetypeAddrInIReg, + ArgVtypeAsScalar, ArgInvalid, } ArgStorage; typedef struct { ArgStorage storage : 8; + MonoType *type; } ArgInfo; struct CallInfo { @@ -38,6 +40,45 @@ struct CallInfo { ArgInfo args [1]; }; +/* Return whenever TYPE represents a vtype with only one scalar member */ +static gboolean +is_scalar_vtype (MonoType *type) +{ + MonoClass *klass; + MonoClassField *field; + gpointer iter; + + if (!MONO_TYPE_ISSTRUCT (type)) + return FALSE; + klass = mono_class_from_mono_type_internal (type); + mono_class_init_internal (klass); + + int size = mono_class_value_size (klass, NULL); + if (size == 0 || size >= 8) + return FALSE; + + iter = NULL; + int nfields = 0; + field = NULL; + while ((field = mono_class_get_fields_internal (klass, &iter))) { + if (field->type->attrs & FIELD_ATTRIBUTE_STATIC) + continue; + nfields ++; + if (nfields > 1) + return FALSE; + if (MONO_TYPE_ISSTRUCT (field->type)) { + if (!is_scalar_vtype (field->type)) + return FALSE; + } else if (!((MONO_TYPE_IS_PRIMITIVE (field->type) || MONO_TYPE_IS_REFERENCE (field->type)))) { + return FALSE; + } + } + + return TRUE; +} + +// WASM ABI: https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md + static ArgStorage get_storage (MonoType *type, gboolean is_return) { @@ -75,14 +116,14 @@ get_storage (MonoType *type, gboolean is_return) /* fall through */ case MONO_TYPE_VALUETYPE: case MONO_TYPE_TYPEDBYREF: { + if (is_scalar_vtype (type)) + return ArgVtypeAsScalar; return is_return ? ArgValuetypeAddrInIReg : ArgValuetypeAddrOnStack; - break; } case MONO_TYPE_VAR: case MONO_TYPE_MVAR: g_assert (mini_is_gsharedvt_type (type)); return ArgGsharedVTOnStack; - break; case MONO_TYPE_VOID: g_assert (is_return); break; @@ -107,7 +148,8 @@ get_call_info (MonoMemPool *mp, MonoMethodSignature *sig) cinfo->gsharedvt = mini_is_gsharedvt_variable_signature (sig); /* return value */ - cinfo->ret.storage = get_storage (mini_get_underlying_type (sig->ret), TRUE); + cinfo->ret.type = mini_get_underlying_type (sig->ret); + cinfo->ret.storage = get_storage (cinfo->ret.type, TRUE); if (sig->hasthis) cinfo->args [0].storage = ArgOnStack; @@ -116,8 +158,10 @@ get_call_info (MonoMemPool *mp, MonoMethodSignature *sig) g_assert (sig->call_convention != MONO_CALL_VARARG); int i; - for (i = 0; i < sig->param_count; ++i) - cinfo->args [i + sig->hasthis].storage = get_storage (mini_get_underlying_type (sig->params [i]), FALSE); + for (i = 0; i < sig->param_count; ++i) { + cinfo->args [i + sig->hasthis].type = mini_get_underlying_type (sig->params [i]); + cinfo->args [i + sig->hasthis].storage = get_storage (cinfo->args [i + sig->hasthis].type, FALSE); + } return cinfo; } @@ -304,7 +348,10 @@ mono_arch_get_llvm_call_info (MonoCompile *cfg, MonoMethodSignature *sig) linfo = mono_mempool_alloc0 (cfg->mempool, sizeof (LLVMCallInfo) + (sizeof (LLVMArgInfo) * n)); - if (mini_type_is_vtype (sig->ret)) { + if (cinfo->ret.storage == ArgVtypeAsScalar) { + linfo->ret.storage = LLVMArgWasmVtypeAsScalar; + linfo->ret.esize = mono_class_value_size (mono_class_from_mono_type_internal (cinfo->ret.type), NULL); + } else if (mini_type_is_vtype (sig->ret)) { /* Vtype returned using a hidden argument */ linfo->ret.storage = LLVMArgVtypeRetAddr; // linfo->vret_arg_index = cinfo->vret_arg_index; @@ -326,6 +373,11 @@ mono_arch_get_llvm_call_info (MonoCompile *cfg, MonoMethodSignature *sig) case ArgGsharedVTOnStack: linfo->args [i].storage = LLVMArgGsharedvtVariable; break; + case ArgVtypeAsScalar: + linfo->args [i].storage = LLVMArgWasmVtypeAsScalar; + linfo->args [i].type = ainfo->type; + linfo->args [i].esize = mono_class_value_size (mono_class_from_mono_type_internal (ainfo->type), NULL); + break; case ArgValuetypeAddrInIReg: g_error ("this is only valid for sig->ret"); break; diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index 4733731656d016..943df2b71b2295 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -667,7 +667,13 @@ typedef enum { /* Vtype returned as an int */ LLVMArgVtypeAsScalar, /* Address to local vtype passed as argument (using register or stack). */ - LLVMArgVtypeAddr + LLVMArgVtypeAddr, + /* + * On WASM, a one element vtype is passed/returned as a scalar with the same + * type as the element. + * esize is the size of the value. + */ + LLVMArgWasmVtypeAsScalar } LLVMArgStorage; typedef struct {