From patchwork Mon Jul 8 16:55:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andi Kleen X-Patchwork-Id: 1958057 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=R7OhVpfM; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WHr5z4vmFz1xpP for ; Tue, 9 Jul 2024 03:02:47 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E2AC9384A413 for ; Mon, 8 Jul 2024 17:02:45 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.11]) by sourceware.org (Postfix) with ESMTPS id 9F18F384A80F for ; Mon, 8 Jul 2024 17:00:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9F18F384A80F 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 9F18F384A80F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=192.198.163.11 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720458057; cv=none; b=Bo/7/yiUuMEN9/fW2dmfH7kMsb7v3rV5tOD2+llZzfTqN51siaOFqy1q76urunUPyIGWOwSXp/tYCgZ/n+ZPZ0mTx6Q99kP8EQQ9mKMjiXWOg2HUoM5iuH/f8qzrevdb80eArWsjV8KpR3u7s0om8wXu/npDnBLFuOB5OoHJhqI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720458057; c=relaxed/simple; bh=pAli+CHxR8EPM/fh8vbVqwVqICpbsoF7KbtvtxyQoeo=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=xKzmCxGatq1WdvG/ZXEnUdvLFvJd10X0bwJcoHqBbp0ocC9yhL9OgFrONDjJSOiqlQ6OTRyF63XLp8g6+AJH4VcgK7bp2pwd2eLKyDq0ySKdYAJsxl9j8KzcHRudcI23Q3dbNDK2+QxuJUj3JR4hZICQf36LDtG+vIVs884P4gA= 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=1720458055; x=1751994055; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=pAli+CHxR8EPM/fh8vbVqwVqICpbsoF7KbtvtxyQoeo=; b=R7OhVpfMsDQkvMnneRI9sbWpfAWushutEQvAGwnuNRftfh0AJ8s7ofBh 60hJa9Q4E9zqg9pSkk4zT8r20CCYIg/A3DKCBXVQddu28p0xMJEENoIUm 85beh9O8Dzp9+f97ipu/LT8yiHvbOqtDAl28Y6mFF/Ww6eRqcicJHRxUp dTjoP2zT+3mtCAfd3Rnm0TV0IyWozrOMdmg9wN+go7R9SgxFXQAmSEwg6 img0MQQ+skwL1Obby7JzK6YRYNsl+r6Gq8XemU3Rne4TKLYzWo84nQR8G tA4WWHSTvmcVbCbpfB67BJ6mXV11HaykBRXtaR/MgSkoeYv/ZZyV1e9XI g==; X-CSE-ConnectionGUID: ErId0WT4SXaZzrZVUsz8YA== X-CSE-MsgGUID: p7lF4F3UQcCXYkvpnF5qLQ== X-IronPort-AV: E=McAfee;i="6700,10204,11127"; a="28279687" X-IronPort-AV: E=Sophos;i="6.09,192,1716274800"; d="scan'208";a="28279687" Received: from orviesa010.jf.intel.com ([10.64.159.150]) by fmvoesa105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jul 2024 10:00:37 -0700 X-CSE-ConnectionGUID: 4cE1YC8bTgqiwOj75rQB4w== X-CSE-MsgGUID: eSX+qDVZQIiwalTivG05Mw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,192,1716274800"; d="scan'208";a="47486299" Received: from tassilo.jf.intel.com ([10.54.38.190]) by orviesa010-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jul 2024 10:00:37 -0700 From: Andi Kleen To: gcc-patches@gcc.gnu.org Cc: richard.guenther@gmail.com, josmyers@redhat.com, polacek@redhat.com, jakub@redhat.com, Andi Kleen Subject: [PATCH v9 07/10] Give better error messages for musttail Date: Mon, 8 Jul 2024 09:55:59 -0700 Message-ID: <20240708170031.1621184-8-ak@linux.intel.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240708170031.1621184-1-ak@linux.intel.com> References: <20240708170031.1621184-1-ak@linux.intel.com> MIME-Version: 1.0 X-Spam-Status: No, score=-11.2 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 autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org 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. In various cases this requires delaying the error until the call is discovered. Also print more information on the failure to the dump file. gcc/ChangeLog: PR83324 * tree-tailcall.cc (maybe_error_musttail): New function. (suitable_for_tail_opt_p): Report error reason. (suitable_for_tail_call_opt_p): Report error reason. (find_tail_calls): Accept basic blocks with abnormal edges. Delay reporting of errors until the call is discovered. Move top level suitability checks to here. (tree_optimize_tail_calls_1): Remove top level checks. --- gcc/tree-tailcall.cc | 187 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 154 insertions(+), 33 deletions(-) diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc index 43e8c25215cb..a68079d4f507 100644 --- a/gcc/tree-tailcall.cc +++ b/gcc/tree-tailcall.cc @@ -40,9 +40,11 @@ along with GCC; see the file COPYING3. If not see #include "tree-eh.h" #include "dbgcnt.h" #include "cfgloop.h" +#include "intl.h" #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 @@ -131,14 +133,20 @@ static tree m_acc, a_acc; static bitmap tailr_arg_needs_copy; +static void maybe_error_musttail (gcall *call, const char *err); + /* Returns false when the function is not suitable for tail call optimization - from some reason (e.g. if it takes variable number of arguments). */ + from some reason (e.g. if it takes variable number of arguments). CALL + is call to report for. */ static bool -suitable_for_tail_opt_p (void) +suitable_for_tail_opt_p (gcall *call) { if (cfun->stdarg) - return false; + { + maybe_error_musttail (call, _("caller uses stdargs")); + return false; + } return true; } @@ -146,35 +154,47 @@ suitable_for_tail_opt_p (void) /* Returns false when the function is not suitable for tail call optimization for some reason (e.g. if it takes variable number of arguments). This test must pass in addition to suitable_for_tail_opt_p in order to make - tail call discovery happen. */ + tail call discovery happen. CALL is call to report error for. */ static bool -suitable_for_tail_call_opt_p (void) +suitable_for_tail_call_opt_p (gcall *call) { tree param; /* alloca (until we have stack slot life analysis) inhibits sibling call optimizations, but not tail recursion. */ if (cfun->calls_alloca) - return false; + { + maybe_error_musttail (call, _("caller uses alloca")); + return false; + } /* If we are using sjlj exceptions, we may need to add a call to _Unwind_SjLj_Unregister at exit of the function. Which means that we cannot do any sibcall transformations. */ if (targetm_common.except_unwind_info (&global_options) == UI_SJLJ && current_function_has_exception_handlers ()) - return false; + { + maybe_error_musttail (call, _("caller uses sjlj exceptions")); + return false; + } /* Any function that calls setjmp might have longjmp called from any called function. ??? We really should represent this properly in the CFG so that this needn't be special cased. */ if (cfun->calls_setjmp) - return false; + { + maybe_error_musttail (call, _("caller uses setjmp")); + return false; + } /* Various targets don't handle tail calls correctly in functions that call __builtin_eh_return. */ if (cfun->calls_eh_return) - return false; + { + maybe_error_musttail (call, _("caller uses __builtin_eh_return")); + return false; + } /* ??? It is OK if the argument of a function is taken in some cases, but not in all cases. See PR15387 and PR19616. Revisit for 4.1. */ @@ -182,7 +202,10 @@ suitable_for_tail_call_opt_p (void) param; param = DECL_CHAIN (param)) if (TREE_ADDRESSABLE (param)) - return false; + { + maybe_error_musttail (call, _("address of caller arguments taken")); + return false; + } return true; } @@ -402,16 +425,42 @@ propagate_through_phis (tree var, edge e) return var; } +/* Report an error for failing to tail convert must call CALL + with error message ERR. Also clear the flag to prevent further + errors. */ + +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); + /* Avoid another error. ??? If there are multiple reasons why tail + calls fail it might be useful to report them all to avoid + whack-a-mole for the user. But currently there is too much + redundancy in the reporting, so keep it simple. */ + gimple_call_set_must_tail (call, false); /* Avoid another error. */ + gimple_call_set_tail (call, false); + } + if (dump_file) + { + print_gimple_stmt (dump_file, call, 0, TDF_SLIM); + fprintf (dump_file, "Cannot convert: %s\n", err); + } +} + /* 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; static vec live_vars_vec; /* Finds tailcalls falling into basic block BB. The list of found tailcalls is - added to the start of RET. When ONLY_MUSTTAIL is set only handle musttail. */ + added to the start of RET. When ONLY_MUSTTAIL is set only handle musttail. + Update OPT_TAILCALLS as output parameter. */ static void -find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) +find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail, + bool &opt_tailcalls) { tree ass_var = NULL_TREE, ret_var, func, param; gimple *stmt; @@ -426,8 +475,23 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) tree var; if (!single_succ_p (bb)) - return; + { + /* If there is an abnormal edge assume it's the only extra one. + Tolerate that case so that we can give better error messages + for musttail later. */ + if (!has_abnormal_or_eh_outgoing_edge_p (bb)) + { + if (dump_file) + fprintf (dump_file, "Basic block %d has extra exit edges\n", + bb->index); + return; + } + if (!cfun->has_musttail) + return; + } + bool bad_stmt = false; + gimple *last_stmt = nullptr; for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi)) { stmt = gsi_stmt (gsi); @@ -441,6 +505,8 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail) || is_gimple_debug (stmt)) continue; + if (!last_stmt) + last_stmt = stmt; /* Check for a call. */ if (is_gimple_call (stmt)) { @@ -448,6 +514,12 @@ 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,19 +534,37 @@ 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; + { + if (dump_file) + { + fprintf (dump_file, "Cannot handle "); + print_gimple_stmt (dump_file, stmt, 0); + } + bad_stmt = true; + if (!cfun->has_musttail) + break; + } } + if (bad_stmt) + return; + if (gsi_end_p (gsi)) { edge_iterator ei; /* Recurse to the predecessors. */ FOR_EACH_EDGE (e, ei, bb->preds) - find_tail_calls (e->src, ret, only_musttail); + find_tail_calls (e->src, ret, only_musttail, opt_tailcalls); return; } + if (!suitable_for_tail_opt_p (call)) + return; + + if (!suitable_for_tail_call_opt_p (call)) + opt_tailcalls = false; + /* If the LHS of our call is not just a simple register or local variable, we can't transform this into a tail or sibling call. This situation happens, in (e.g.) "*p = foo()" where foo returns a @@ -489,13 +579,30 @@ 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, _("return value in memory")); + return; + } + + if (cfun->calls_setjmp) + { + maybe_error_musttail (call, _("caller uses setjmp")); + 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) - && !stmt_can_throw_external (cfun, stmt)) + if ((stmt_could_throw_p (cfun, stmt) + && !stmt_can_throw_external (cfun, stmt)) || !single_succ_p (bb)) + { + if (stmt == last_stmt) + maybe_error_musttail (call, + _("call may throw exception that does not propagate")); + else + maybe_error_musttail (call, + _("code between call and return")); 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 +631,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 +715,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 +724,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 +770,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 +832,26 @@ 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 uses return slot")); + 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) @@ -1112,17 +1238,12 @@ tree_optimize_tail_calls_1 (bool opt_tailcalls, bool only_mustcall) tree param; edge_iterator ei; - if (!suitable_for_tail_opt_p ()) - return 0; - if (opt_tailcalls) - opt_tailcalls = suitable_for_tail_call_opt_p (); - FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds) { /* Only traverse the normal exits, i.e. those that end with return statement. */ if (safe_is_a (*gsi_last_bb (e->src))) - find_tail_calls (e->src, &tailcalls, only_mustcall); + find_tail_calls (e->src, &tailcalls, only_mustcall, opt_tailcalls); } if (live_vars)