[mono][aot] Add a pass to remove empty finally clauses. (#53482)
authorZoltan Varga <vargaz@gmail.com>
Tue, 1 Jun 2021 16:50:24 +0000 (12:50 -0400)
committerGitHub <noreply@github.com>
Tue, 1 Jun 2021 16:50:24 +0000 (12:50 -0400)
This can remove the try-finally added for list iterators, i.e.:

        var l = new List<int> ();
        foreach (var i in l) {
        }

Where the finally clause is:

      IL_0040:  ldloca.s   V_3
      IL_0042:  constrained. [System.Collections]System.Collections.Generic.List`1/Enumerator<int32>
      IL_0048:  callvirt   instance void [System.Runtime]System.IDisposable::Dispose()
      IL_004d:  endfinally

and the Dispose method for List`1/Enumerator is empty.

src/mono/mono/mini/method-to-ir.c
src/mono/mono/mini/mini-llvm.c
src/mono/mono/mini/mini.c
src/mono/mono/mini/mini.h

index 238a4db..a10dbd7 100644 (file)
@@ -6219,6 +6219,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
                        cfg->spvars = g_hash_table_new (NULL, NULL);
                        cfg->exvars = g_hash_table_new (NULL, NULL);
                }
+               cfg->clause_is_dead = mono_mempool_alloc0 (cfg->mempool, sizeof (gboolean) * header->num_clauses);
+
                /* handle exception clauses */
                for (i = 0; i < header->num_clauses; ++i) {
                        MonoBasicBlock *try_bb;
@@ -6475,9 +6477,16 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
        if (is_virtual_call) {
                MonoInst *arg_ins;
 
-               NEW_ARGLOAD (cfg, arg_ins, 0);
-               MONO_ADD_INS (cfg->cbb, arg_ins);
-               MONO_EMIT_NEW_CHECK_THIS (cfg, arg_ins->dreg);
+               //
+               // This is just a hack to avoid checks in empty methods which could get inlined
+               // into finally clauses preventing the removal of empty finally clauses, since all
+               // variables in finally clauses are marked volatile so the check can't be removed
+               //
+               if (!(cfg->llvm_only && m_class_is_valuetype (method->klass) && header->code_size == 1 && header->code [0] == CEE_RET)) {
+                       NEW_ARGLOAD (cfg, arg_ins, 0);
+                       MONO_ADD_INS (cfg->cbb, arg_ins);
+                       MONO_EMIT_NEW_CHECK_THIS (cfg, arg_ins->dreg);
+               }
        }
 
        if (cfg->llvm_only && cfg->interp)
index d8424dc..88156cb 100644 (file)
@@ -11792,6 +11792,10 @@ emit_method_inner (EmitContext *ctx)
        if (ctx->llvm_only && !ctx->cfg->interp_entry_only) {
                size_t group_index = 0;
                while (group_index < cfg->header->num_clauses) {
+                       if (cfg->clause_is_dead [group_index]) {
+                               group_index ++;
+                               continue;
+                       }
                        int count = 0;
                        size_t cursor = group_index;
                        while (cursor < cfg->header->num_clauses &&
@@ -12028,9 +12032,12 @@ after_codegen:
                        g_free (name);
                }
 
-               //LLVMDumpValue (ctx->lmethod);
-               //int err = LLVMVerifyFunction (ctx->lmethod, LLVMPrintMessageAction);
-               //g_assert (err == 0);
+               /*
+               int err = LLVMVerifyFunction (ctx->lmethod, LLVMPrintMessageAction);
+               if (err != 0)
+                       LLVMDumpValue (ctx->lmethod);
+               g_assert (err == 0);
+               */
        } else {
                //LLVMVerifyFunction (method, 0);
                llvm_jit_finalize_method (ctx);
index 29bb69c..c8665a0 100644 (file)
@@ -2368,10 +2368,16 @@ create_jit_info (MonoCompile *cfg, MonoMethod *method_to_compile)
                        printf ("Number of try block holes %d\n", num_holes);
        }
 
-       if (COMPILE_LLVM (cfg))
+       if (COMPILE_LLVM (cfg)) {
                num_clauses = cfg->llvm_ex_info_len;
-       else
+       } else {
                num_clauses = header->num_clauses;
+               int dead_clauses = 0;
+               for (int i = 0; i < header->num_clauses; ++i)
+                       if (cfg->clause_is_dead [i])
+                               dead_clauses ++;
+               num_clauses -= dead_clauses;
+       }
 
        if (cfg->method->dynamic)
                jinfo = (MonoJitInfo *)g_malloc0 (mono_jit_info_size (flags, num_clauses, num_holes));
@@ -2509,15 +2515,18 @@ create_jit_info (MonoCompile *cfg, MonoMethod *method_to_compile)
        if (COMPILE_LLVM (cfg)) {
                if (num_clauses)
                        memcpy (&jinfo->clauses [0], &cfg->llvm_ex_info [0], num_clauses * sizeof (MonoJitExceptionInfo));
-       } else if (header->num_clauses) {
-               int i;
-
-               for (i = 0; i < header->num_clauses; i++) {
+       } else {
+               int eindex = 0;
+               for (int i = 0; i < header->num_clauses; i++) {
                        MonoExceptionClause *ec = &header->clauses [i];
-                       MonoJitExceptionInfo *ei = &jinfo->clauses [i];
+                       MonoJitExceptionInfo *ei = &jinfo->clauses [eindex];
                        MonoBasicBlock *tblock;
                        MonoInst *exvar;
 
+                       if (cfg->clause_is_dead [i])
+                               continue;
+                       eindex ++;
+
                        ei->flags = ec->flags;
 
                        if (G_UNLIKELY (cfg->verbose_level >= 4))
@@ -2878,6 +2887,84 @@ mono_insert_branches_between_bblocks (MonoCompile *cfg)
 }
 
 static void
+remove_empty_finally_pass (MonoCompile *cfg)
+{
+       MonoBasicBlock *bb;
+       MonoInst *ins;
+       gboolean remove_call_handler = FALSE;
+
+       // FIXME: other configurations
+       if (!cfg->llvm_only)
+               return;
+
+       for (int i = 0; i < cfg->header->num_clauses; ++i) {
+               MonoExceptionClause *clause = &cfg->header->clauses [i];
+
+               if (clause->flags == MONO_EXCEPTION_CLAUSE_FINALLY) {
+                       MonoInst *first, *last;
+
+                       bb = cfg->cil_offset_to_bb [clause->handler_offset];
+                       g_assert (bb);
+
+                       /* Support only 1 bb for now */
+                       first = mono_bb_first_inst (bb, 0);
+                       if (first->opcode != OP_START_HANDLER)
+                               break;
+
+                       gboolean empty = TRUE;
+                       while (TRUE) {
+                               if (bb->out_count > 1) {
+                                       empty = FALSE;
+                                       break;
+                               }
+                               MONO_BB_FOR_EACH_INS (bb, ins) {
+                                       if (!(ins->opcode == OP_START_HANDLER || ins->opcode == OP_ENDFINALLY || MONO_INS_HAS_NO_SIDE_EFFECT (ins))) {
+                                               empty = FALSE;
+                                               break;
+                                       }
+                               }
+                               if (!empty)
+                                       break;
+                               if (bb->out_count == 0)
+                                       break;
+                               if (mono_bb_last_inst (bb, 0)->opcode == OP_ENDFINALLY)
+                                       break;
+                               bb = bb->out_bb [0];
+                       }
+                       if (empty) {
+                               bb->flags &= ~BB_EXCEPTION_HANDLER;
+                               NULLIFY_INS (first);
+                               last = mono_bb_last_inst (bb, 0);
+                               if (last->opcode == OP_ENDFINALLY)
+                                       NULLIFY_INS (last);
+                               if (cfg->verbose_level > 1)
+                                       g_print ("removed empty finally clause %d.\n", i);
+                               cfg->clause_is_dead [i] = TRUE;
+                               remove_call_handler = TRUE;
+                       }
+               }
+       }
+
+       if (remove_call_handler) {
+               /* Remove OP_CALL_HANDLER opcodes pointing to the removed finally blocks */
+               for (bb = cfg->bb_entry; bb; bb = bb->next_bb) {
+                       MonoInst *handler_ins = NULL;
+                       MONO_BB_FOR_EACH_INS (bb, ins) {
+                               if (ins->opcode == OP_CALL_HANDLER && ins->inst_target_bb && !(ins->inst_target_bb->flags & BB_EXCEPTION_HANDLER)) {
+                                       handler_ins = ins;
+                                       break;
+                               }
+                       }
+                       if (handler_ins) {
+                               handler_ins->opcode = OP_BR;
+                               for (ins = handler_ins->next; ins; ins = ins->next)
+                                       NULLIFY_INS (ins);
+                       }
+               }
+       }
+}
+
+static void
 init_backend (MonoBackend *backend)
 {
 #ifdef MONO_ARCH_NEED_GOT_VAR
@@ -3492,6 +3579,8 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts
                mono_cfg_dump_ir (cfg, "if_conversion");
        }
 
+       remove_empty_finally_pass (cfg);
+
        mono_threads_safepoint ();
 
        MONO_TIME_TRACK (mono_jit_stats.jit_bb_ordering, mono_bb_ordering (cfg));
index cf89d29..eea7742 100644 (file)
@@ -1606,6 +1606,8 @@ typedef struct {
        /* pointer to context datastructure used for graph dumping */
        MonoGraphDumper *gdump_ctx;
 
+       gboolean *clause_is_dead;
+
        /* Stats */
        int stat_allocate_var;
        int stat_locals_stack_size;