From: Bernhard Urban-Forster Date: Thu, 19 Dec 2019 01:05:57 +0000 (+0100) Subject: [interp] add guard that a local hasn't been modified for stlocfld fusion (mono/mono... X-Git-Tag: submit/tizen/20210909.063632~10331^2~5^2~70 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=53d45002e9670b855be17556fa3d629b0a83213a;p=platform%2Fupstream%2Fdotnet%2Fruntime.git [interp] add guard that a local hasn't been modified for stlocfld fusion (mono/mono#18129) [interp] add guard that a local hasn't been modified for stlocfld fusion Fixes https://github.com/mono/mono/issues/18120 Commit migrated from https://github.com/mono/mono/commit/e7b98d3587fddd51db69baddb370321cca521d60 --- diff --git a/src/mono/mono/mini/iltests.il b/src/mono/mono/mini/iltests.il index e361af3..4ee7e2c 100644 --- a/src/mono/mono/mini/iltests.il +++ b/src/mono/mono/mini/iltests.il @@ -3358,6 +3358,97 @@ wrong: ret } + .class nested private auto ansi beforefieldinit R1_class + { + .field public int32 test_field + + .method public hidebysig specialname rtspecialname instance default void '.ctor' () cil managed + { + ret + } + } + + .method public hidebysig static int32 test_3_ldstloc_opt () cil managed + { + .locals init ( + class Tests/R1_class v_0, + int32 v_1, + class Tests/R1_class v_2 + ) + + /* allocate first object */ + newobj instance void class Tests/R1_class::.ctor () + stloc.0 + ldloc.0 + stloc.2 + + br L3 + /* dead code for interp opt barrier */ + ldc.i4.1 + ldc.i4.1 + add + pop +L3: + /* load first object on the stack */ + ldloc.0 + + ldc.i4.3 + stloc.1 + ldloc.1 + + /* allocate second object */ + newobj instance void class Tests/R1_class::.ctor () + /* overwrite first object at loc0 with second object */ + stloc.0 + + /* should store '3' into first object. + * in the buggy case it stores '3' into the second object because it delays the load from loc.0 */ + stfld int32 Tests/R1_class::test_field + + /* load first object on the stack */ + ldloc.2 + /* should read '3' */ + ldfld int32 Tests/R1_class::test_field + ret + } + + .method public hidebysig static void ldstarg_opt_helper (Tests/R1_class a) cil managed + { + ldarg.0 + + /* allocate second object */ + newobj instance void class Tests/R1_class::.ctor () + dup + ldc.i4.1 + /* stores '1' in the field */ + stfld int32 Tests/R1_class::test_field + /* overwrite first argument */ + starg 0 + + ldc.i4.3 + /* stores '3' in the field */ + stfld int32 Tests/R1_class::test_field /* might get fused with ldarg.0 at start of method */ + ret + } + + .method public hidebysig static int32 test_3_ldstarg_opt () cil managed + { + /* allocate first object */ + newobj instance void class Tests/R1_class::.ctor () + dup + ldc.i4.s 0x1337 + /* stores '0x1337' in the field */ + stfld int32 Tests/R1_class::test_field + + dup + /* passing first object */ + call void class Tests::ldstarg_opt_helper (Tests/R1_class) + /* should read '3' from first object */ + ldfld int32 Tests/R1_class::test_field + ret + } + + .method public hidebysig static int32 test_10_rconv_to_u8_ovf_un() cil managed { // Code size 20 (0x14) diff --git a/src/mono/mono/mini/interp/mintops.h b/src/mono/mono/mini/interp/mintops.h index 9e3c3b6..4164733 100644 --- a/src/mono/mono/mini/interp/mintops.h +++ b/src/mono/mono/mini/interp/mintops.h @@ -58,6 +58,8 @@ typedef enum { #define MINT_IS_STLOC(op) ((op) >= MINT_STLOC_I1 && (op) <= MINT_STLOC_VT) #define MINT_IS_MOVLOC(op) ((op) >= MINT_MOVLOC_1 && (op) <= MINT_MOVLOC_VT) #define MINT_IS_STLOC_NP(op) ((op) >= MINT_STLOC_NP_I4 && (op) <= MINT_STLOC_NP_O) +#define MINT_IS_LDARG(op) ((op) >= MINT_LDARG_I1 && (op) <= MINT_LDARG_VT) +#define MINT_IS_STARG(op) ((op) >= MINT_STARG_I1 && (op) <= MINT_STARG_VT) #define MINT_IS_CONDITIONAL_BRANCH(op) ((op) >= MINT_BRFALSE_I4 && (op) <= MINT_BLT_UN_R8_S) #define MINT_IS_CALL(op) ((op) >= MINT_CALL && (op) <= MINT_JIT_CALL) #define MINT_IS_PATCHABLE_CALL(op) ((op) >= MINT_CALL && (op) <= MINT_VCALL) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 9379f37..0da8b1c 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -56,17 +56,19 @@ typedef struct #define STACK_VALUE_NONE 0 #define STACK_VALUE_LOCAL 1 -#define STACK_VALUE_I4 2 -#define STACK_VALUE_I8 3 +#define STACK_VALUE_ARG 2 +#define STACK_VALUE_I4 3 +#define STACK_VALUE_I8 4 // StackValue contains data to construct an InterpInst that is equivalent with the contents -// of the stack slot / local. +// of the stack slot / local / argument. typedef struct { - // Indicates the type of the stored information. It can be a local or a constant + // Indicates the type of the stored information. It can be a local, argument or a constant int type; // Holds the local index or the actual constant value union { int local; + int arg; gint32 i; gint64 l; }; @@ -75,7 +77,7 @@ typedef struct { typedef struct { // This indicates what is currently stored in this stack slot. This can be a constant - // or the copy of a local. + // or the copy of a local / argument. StackValue val; // The instruction that pushed this stack slot. If ins is null, we can't remove the usage // of the stack slot, because we can't clear the instruction that set it. @@ -6656,6 +6658,18 @@ clear_stack_content_info_for_local (StackContentInfo *start, StackContentInfo *e } } +// The value of argument has changed. This means the contents of the stack where the +// argument was loaded, no longer contain the value of the argument. Clear them. +static void +clear_stack_content_info_for_argument (StackContentInfo *start, StackContentInfo *end, int argument) +{ + StackContentInfo *si; + for (si = start; si < end; si++) { + if (si->val.type == STACK_VALUE_ARG && si->val.arg == argument) + si->val.type = STACK_VALUE_NONE; + } +} + // The value of local has changed. This means we can no longer assume that any other local // is a copy of this local. static void @@ -7114,7 +7128,7 @@ retry: } else { locals [dest_local].type = STACK_VALUE_NONE; } - } else if (sp->val.type == STACK_VALUE_NONE) { + } else if (sp->val.type == STACK_VALUE_NONE || sp->val.type == STACK_VALUE_ARG) { locals [dest_local].type = STACK_VALUE_NONE; } else { g_assert (sp->val.type == STACK_VALUE_I4 || sp->val.type == STACK_VALUE_I8); @@ -7183,6 +7197,15 @@ retry: // propagated instruction, so we remove the top of stack dependency sp [-1].ins = NULL; sp++; + } else if (MINT_IS_LDARG (ins->opcode)) { + sp->ins = ins; + sp->val.type = STACK_VALUE_ARG; + sp->val.arg = ins->opcode == MINT_LDARG_P0 ? 0 : ins->data [0]; + sp++; + } else if (MINT_IS_STARG (ins->opcode)) { + int dest_arg = ins->data [0]; + sp--; + clear_stack_content_info_for_argument (stack, sp, dest_arg); } else if (ins->opcode >= MINT_BOX && ins->opcode <= MINT_BOX_NULLABLE) { int offset = ins->data [1]; // Clear the stack slot that is boxed @@ -7241,30 +7264,29 @@ retry: ins = interp_fold_binop (td, sp, ins); sp--; } else if (ins->opcode >= MINT_STFLD_I1 && ins->opcode <= MINT_STFLD_P && (mono_interp_opt & INTERP_OPT_SUPER_INSTRUCTIONS)) { - if (sp [-2].ins) { - InterpInst *obj_ins = sp [-2].ins; - if (obj_ins->opcode == MINT_LDLOC_O) { - int loc_index = obj_ins->data [0]; + StackContentInfo *src = &sp [-2]; + if (src->ins) { + if (src->val.type == STACK_VALUE_LOCAL) { + int loc_index = src->val.local; int fld_offset = ins->data [0]; int mt = ins->opcode - MINT_STFLD_I1; ins = interp_insert_ins (td, ins, MINT_STLOCFLD_I1 + mt); ins->data [0] = loc_index; ins->data [1] = fld_offset; + local_ref_count [loc_index]++; interp_clear_ins (td, ins->prev); - interp_clear_ins (td, obj_ins); + interp_clear_ins (td, src->ins); mono_interp_stats.super_instructions++; mono_interp_stats.killed_instructions++; - } else if (obj_ins->opcode == MINT_LDARG_O || obj_ins->opcode == MINT_LDARG_P0) { - int arg_index = 0; + } else if (src->val.type == STACK_VALUE_ARG) { + int arg_index = src->val.arg; int fld_offset = ins->data [0]; int mt = ins->opcode - MINT_STFLD_I1; - if (obj_ins->opcode == MINT_LDARG_O) - arg_index = obj_ins->data [0]; ins = interp_insert_ins (td, ins, MINT_STARGFLD_I1 + mt); ins->data [0] = arg_index; ins->data [1] = fld_offset; interp_clear_ins (td, ins->prev); - interp_clear_ins (td, obj_ins); + interp_clear_ins (td, src->ins); mono_interp_stats.super_instructions++; mono_interp_stats.killed_instructions++; }