From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.12]) by sourceware.org (Postfix) with ESMTPS id 980E53816B1C for ; Wed, 5 Jun 2024 04:52:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 980E53816B1C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.intel.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=linux.intel.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 980E53816B1C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=198.175.65.12 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1717563126; cv=none; b=I85nEHLdR4UH/pqDOaqsFI5yCgifWUsnDDYAVowVbZOVF9aYLESeydfxVFg0MkxwP4oU/cgn8nptWXg39wg92O6y9d1xOywnVSqY8dC2duqpv2p4nodGQamxDF/gYdsieECxGUhTlrZyoB6Lc1K0pZqUGfP796rNje9gXM/VsWI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1717563126; c=relaxed/simple; bh=IY6hlfFg1V+e/yJNlhehgK9M2nv/kRGi6N6QHvxs7wQ=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=YDsj/5uU6nQsQDVXoqQT8xK3aOIf+paepYOe6h2/ZnlIlAxCi5TnAwK3+GKb+JalwFtruGIslPc9pblTuzEhKcmHLS1IIqDC4kN+XQvp4cVdOI8NKAPFnZ3uzlFJhNxlRz0Nbb2TQiMDEUisZzsVTyowGDgs4YjLzemV6xrRV0o= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1717563124; x=1749099124; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=IY6hlfFg1V+e/yJNlhehgK9M2nv/kRGi6N6QHvxs7wQ=; b=Fg6h7YIrn0YJvBQBz9nz+XGBqRvmR+J0LfaeCnB+VV36HdVYISGFZvtY o/n2vSW6/V9mp/GA1L+IzOliOF+xt9VizFUQ2xATdSh97rnm1yCYRP/sn OsJzMH9EEtZw/xPjcqX8SaKwaVtzoriXsJyADmLM3E3ezg3fuLr4EBkee eIxkGu/iCvMXZM57/BYyD19976nbJN/I0+tEJfqCYqNbN11h/x8xhGx7u hDTPOfwQ66JsmdMy4IWiqeX1QxmnpOrWM9szd0BGxHw6PPyuJAqioSwlg Py29vs4bFmF2L/eiB3bw4HjoDAWArup1G/bZ1k0IDPer+lMfK4sKcOrhN Q==; X-CSE-ConnectionGUID: TPHkiuQrQay/4n24MB2QvA== X-CSE-MsgGUID: p8CSpbe6QPiJnk+pJmO+dw== X-IronPort-AV: E=McAfee;i="6600,9927,11093"; a="25552256" X-IronPort-AV: E=Sophos;i="6.08,215,1712646000"; d="scan'208";a="25552256" Received: from orviesa009.jf.intel.com ([10.64.159.149]) by orvoesa104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jun 2024 21:52:03 -0700 X-CSE-ConnectionGUID: QSquRXVaQRuLks2IgnzaPA== X-CSE-MsgGUID: Mm4eWcYKSNi5t0+BLX71ng== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,215,1712646000"; d="scan'208";a="37519442" Received: from tassilo.jf.intel.com (HELO tassilo) ([10.54.38.190]) by orviesa009-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jun 2024 21:52:04 -0700 Date: Tue, 4 Jun 2024 21:52:02 -0700 From: Andi Kleen To: gcc-patches@gcc.gnu.org Cc: richard.guenther@gmail.com, nathan@acm.org, josmyers@redhat.com, richard.sandiford@arm.com, jason@redhat.com Subject: Re: [PATCH v6 7/8] Give better error messages for musttail Message-ID: References: <20240521143203.2893096-1-ak@linux.intel.com> <20240521143203.2893096-8-ak@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240521143203.2893096-8-ak@linux.intel.com> X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: [I slightly improve the patch covering a few more cases where tree-tailcall gives up, especially with -O1 and -Os. Here's the updated version.] Give better error messages for musttail When musttail is set, make tree-tailcall give error messages when it cannot handle a call. This avoids vague "other reasons" error messages later at expand time when it sees a musttail function not marked tail call. gcc/ChangeLog: * tree-tailcall.cc (maybe_error_musttail): Add. (bb_get_succ_edge_count): Add. (find_tail_calls): Add error messages. Keep searching for basic blocks with multiple BBs if all but one is EH only. diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc index 094856de22ef..7268e8138529 100644 --- a/gcc/tree-tailcall.cc +++ b/gcc/tree-tailcall.cc @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3. If not see #include "common/common-target.h" #include "ipa-utils.h" #include "tree-ssa-live.h" +#include "diagnostic-core.h" /* The file implements the tail recursion elimination. It is also used to analyze the tail calls in general, passing the results to the rtl level @@ -402,6 +403,36 @@ propagate_through_phis (tree var, edge e) return var; } +/* Report an error for failing to tail convert must call CALL + with error message ERR. */ + +static void +maybe_error_musttail (gcall *call, const char *err) +{ + if (gimple_call_must_tail_p (call)) + { + error_at (call->location, "cannot tail-call: %s", err); + gimple_call_set_must_tail (call, false); /* Avoid another error. */ + gimple_call_set_tail (call, false); + } +} + +/* Count succ edges for BB and return in NUM_OTHER and NUM_EH. */ + +static void +bb_get_succ_edge_count (basic_block bb, int &num_other, int &num_eh) +{ + edge e; + edge_iterator ei; + num_eh = 0; + num_other = 0; + FOR_EACH_EDGE (e, ei, bb->succs) + if (e->flags & EDGE_EH) + num_eh++; + else + num_other++; +} + /* Argument for compute_live_vars/live_vars_at_stmt and what compute_live_vars returns. Computed lazily, but just once for the function. */ static live_vars_map *live_vars; @@ -426,8 +457,16 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) tree var; if (!single_succ_p (bb)) - return; + { + int num_eh, num_other; + bb_get_succ_edge_count (bb, num_eh, num_other); + /* Allow a single EH edge so that we can give a better + error message later. */ + if (!(num_eh == 1 && num_other == 1)) + return; + } + bool bad_stmt = false; for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi)) { stmt = gsi_stmt (gsi); @@ -448,6 +487,11 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) /* Handle only musttail calls when not optimizing. */ if (only_musttail && !gimple_call_must_tail_p (call)) return; + if (bad_stmt) + { + maybe_error_musttail (call, "Memory reference or volatile after call"); + return; + } ass_var = gimple_call_lhs (call); break; } @@ -462,7 +506,9 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) /* If the statement references memory or volatile operands, fail. */ if (gimple_references_memory_p (stmt) || gimple_has_volatile_ops (stmt)) - return; + { + bad_stmt = true; + } } if (gsi_end_p (gsi)) @@ -489,13 +535,21 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) if (ass_var && !is_gimple_reg (ass_var) && !auto_var_in_fn_p (ass_var, cfun->decl)) - return; + { + maybe_error_musttail (call, "complex return value"); + return; + } /* If the call might throw an exception that wouldn't propagate out of cfun, we can't transform to a tail or sibling call (82081). */ - if (stmt_could_throw_p (cfun, stmt) + if ((stmt_could_throw_p (cfun, stmt) && !stmt_can_throw_external (cfun, stmt)) + || !single_succ_p (bb)) + { + maybe_error_musttail (call, + "call may throw exception that does not propagate"); return; + } /* If the function returns a value, then at present, the tail call must return the same type of value. There is conceptually a copy @@ -524,7 +578,10 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) if (result_decl && may_be_aliased (result_decl) && ref_maybe_used_by_stmt_p (call, result_decl, false)) - return; + { + maybe_error_musttail (call, "tail call must be same type"); + return; + } /* We found the call, check whether it is suitable. */ tail_recursion = false; @@ -605,6 +662,7 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) { if (local_live_vars) BITMAP_FREE (local_live_vars); + maybe_error_musttail (call, "call invocation refers to locals"); return; } else @@ -613,6 +671,7 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) if (bitmap_bit_p (local_live_vars, *v)) { BITMAP_FREE (local_live_vars); + maybe_error_musttail (call, "call invocation refers to locals"); return; } } @@ -658,17 +717,21 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) continue; if (gimple_code (stmt) != GIMPLE_ASSIGN) - return; + { + maybe_error_musttail (call, "unhandled code after call"); + return; + } /* This is a gimple assign. */ par ret = process_assignment (as_a (stmt), gsi, &tmp_m, &tmp_a, &ass_var, to_move_defs); - if (ret == FAIL) - return; + if (ret == FAIL || (ret == TRY_MOVE && !tail_recursion)) + { + maybe_error_musttail (call, "return value changed after call"); + return; + } else if (ret == TRY_MOVE) { - if (! tail_recursion) - return; /* Do not deal with checking dominance, the real fix is to do path isolation for the transform phase anyway, removing the need to compute the accumulators with new stmts. */ @@ -716,16 +779,25 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) if (ret_var && (ret_var != ass_var && !(is_empty_type (TREE_TYPE (ret_var)) && !ass_var))) - return; + { + maybe_error_musttail (call, "call must be the same type"); + return; + } /* If this is not a tail recursive call, we cannot handle addends or multiplicands. */ if (!tail_recursion && (m || a)) - return; + { + maybe_error_musttail (call, "operations after non tail recursive call"); + return; + } /* For pointers only allow additions. */ if (m && POINTER_TYPE_P (TREE_TYPE (DECL_RESULT (current_function_decl)))) - return; + { + maybe_error_musttail (call, "tail recursion with pointers can only use additions"); + return; + } /* Move queued defs. */ if (tail_recursion)