[mono][aot] Add a pass to remove empty finally clauses. (#53482)

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

@@ -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)

src/mono/mono/mini/mini-llvm.c

@@ -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 @@ emit_method_inner (EmitContext *ctx)
 			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);

src/mono/mono/mini/mini.c

@@ -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))
@@ -2877,6 +2886,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)
 {
@@ -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));

src/mono/mono/mini/mini.h

@@ -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;