From a202b890aadc1417439391542a02a3098b2b8d0a Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Fri, 3 Dec 2021 21:31:31 -0500 Subject: [PATCH 1/2] Revert "[mono][wasm] Fix the passing/returning of small vtypes. (#62299)" This reverts commit 915ee6db9cb871bb2ef32698015bca0f2c895ce8. This causes test failures in the System.Collections.Immutable testsuite on wasm. --- 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, 8 insertions(+), 100 deletions(-) diff --git a/src/mono/mono/mini/mini-llvm.c b/src/mono/mono/mini/mini-llvm.c index 5830c336da380a..0d19ee09a9bb4e 100644 --- a/src/mono/mono/mini/mini-llvm.c +++ b/src/mono/mono/mini/mini-llvm.c @@ -1568,10 +1568,6 @@ 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; } @@ -1689,10 +1685,6 @@ 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); @@ -3833,7 +3825,6 @@ emit_entry_bb (EmitContext *ctx, LLVMBuilderRef builder) char *name; pindex = ainfo->pindex; - LLVMValueRef arg = LLVMGetParam (ctx->lmethod, pindex); switch (ainfo->storage) { case LLVMArgVtypeInReg: @@ -3892,16 +3883,6 @@ 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); @@ -4435,10 +4416,6 @@ 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]); @@ -4588,11 +4565,6 @@ 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 */ @@ -5719,8 +5691,7 @@ process_bb (EmitContext *ctx, MonoBasicBlock *bb) switch (linfo->ret.storage) { case LLVMArgNormal: case LLVMArgVtypeInReg: - case LLVMArgVtypeAsScalar: - case LLVMArgWasmVtypeAsScalar: { + case LLVMArgVtypeAsScalar: { LLVMTypeRef ret_type = LLVMGetReturnType (LLVMGetElementType (LLVMTypeOf (method))); LLVMValueRef retval = LLVMGetUndef (ret_type); gboolean src_in_reg = FALSE; @@ -5777,10 +5748,6 @@ 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; @@ -12196,7 +12163,6 @@ 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 7e2c7bb1047dcd..45910e1c2b002f 100644 --- a/src/mono/mono/mini/mini-wasm.c +++ b/src/mono/mono/mini/mini-wasm.c @@ -23,13 +23,11 @@ typedef enum { ArgValuetypeAddrOnStack, ArgGsharedVTOnStack, ArgValuetypeAddrInIReg, - ArgVtypeAsScalar, ArgInvalid, } ArgStorage; typedef struct { ArgStorage storage : 8; - MonoType *type; } ArgInfo; struct CallInfo { @@ -40,45 +38,6 @@ 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) { @@ -116,14 +75,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; @@ -148,8 +107,7 @@ get_call_info (MonoMemPool *mp, MonoMethodSignature *sig) cinfo->gsharedvt = mini_is_gsharedvt_variable_signature (sig); /* return value */ - cinfo->ret.type = mini_get_underlying_type (sig->ret); - cinfo->ret.storage = get_storage (cinfo->ret.type, TRUE); + cinfo->ret.storage = get_storage (mini_get_underlying_type (sig->ret), TRUE); if (sig->hasthis) cinfo->args [0].storage = ArgOnStack; @@ -158,10 +116,8 @@ 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].type = mini_get_underlying_type (sig->params [i]); - cinfo->args [i + sig->hasthis].storage = get_storage (cinfo->args [i + sig->hasthis].type, FALSE); - } + for (i = 0; i < sig->param_count; ++i) + cinfo->args [i + sig->hasthis].storage = get_storage (mini_get_underlying_type (sig->params [i]), FALSE); return cinfo; } @@ -348,10 +304,7 @@ mono_arch_get_llvm_call_info (MonoCompile *cfg, MonoMethodSignature *sig) linfo = mono_mempool_alloc0 (cfg->mempool, sizeof (LLVMCallInfo) + (sizeof (LLVMArgInfo) * n)); - 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)) { + 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; @@ -373,11 +326,6 @@ 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 943df2b71b2295..4733731656d016 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -667,13 +667,7 @@ typedef enum { /* Vtype returned as an int */ LLVMArgVtypeAsScalar, /* Address to local vtype passed as argument (using register or stack). */ - 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 + LLVMArgVtypeAddr } LLVMArgStorage; typedef struct { From 8f3e016b86c6a5f200f974929a364f28d43f69a5 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Sat, 4 Dec 2021 18:01:35 -0500 Subject: [PATCH 2/2] Reenable tests. --- src/libraries/tests.proj | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/libraries/tests.proj b/src/libraries/tests.proj index a0e088b9b484a9..6f0c5c31b43294 100644 --- a/src/libraries/tests.proj +++ b/src/libraries/tests.proj @@ -20,17 +20,6 @@ - - - - - - - - - - -