https://gcc.gnu.org/g:698e337bec3a36230c72816fcb82f1a239e64eba

commit r15-8878-g698e337bec3a36230c72816fcb82f1a239e64eba
Author: Jakub Jelinek <ja...@redhat.com>
Date:   Tue Mar 25 09:36:41 2025 +0100

    tailc: Only diagnose musttail failures during tailc or musttail passes 
[PR119376]
    
    The following testcases FAIL because musttail failures are diagnosed
    not just in the tailc or musttail passes, but also during the tailr1
    and tailr2.
    tailr1 pass is before IPA and in the testcases eh cleanup has not
    cleaned up the IL sufficiently yet to make the musttail calls pass,
    even tailr2 could be too early.
    
    The following patch does that only during the tailc pass, and if that
    pass is not actually executed, during musttail pass.
    To do it only in the tailc pass, I chose to pass a new bool flag, because
    while we have the opt_tailcalls argument, it is actually passed by reference
    to find_tail_calls and sometimes cleared during that.
    musttail calls when the new DIAG_MUSTTAIL flag is not set are handled like
    any other calls, we simply silently punt on those if they can't be turned
    into tail calls.
    
    Furthermore, I had to tweak the musttail pass gate.  Previously it was
    !flag_optimize_sibling_calls && f->has_musttail.  The problem is that
    gate of tailr and tailc passes is
    flag_optimize_sibling_calls != 0 && dbg_cnt (tail_call)
    and furthermore, tailc pass is only in the normal optimization queue,
    so only if not -O0 or -Og.  So when one would use tail_call dbg_cnt
    with some limit, or when e.g. using -foptimize-sibling-calls with -O0 or
    -Og, nothing would actually diagnose invalid musttail calls or set tail call
    flags on those if they are ok.  I could insert a new PROP_ flag on whether
    musttail has been handled by tailc pass, but given that we have the
    cfun->has_musttail flag already and nothing after tailc/musttail passes uses
    it, I think it is easier to just clear the flag when musttail failures are
    diagnosed and correct ones have [[tail call]] flag added.  Expansion will
    then only look at the [[tail call]] flag, it could even at the [[must tail
    call]] flag, but I don't see a point to check cfun->has_musttail.
    
    2025-03-25  Jakub Jelinek  <ja...@redhat.com>
    
            PR ipa/119376
            * tree-tailcall.cc (suitable_for_tail_opt_p): Add DIAG_MUSTTAIL
            argument, propagate it down to maybe_error_musttail.
            (suitable_for_tail_call_opt_p): Likewise.
            (maybe_error_musttail): Add DIAG_MUSTTAIL argument.  Don't emit 
error
            for gimple_call_must_tail_p calls if it is false.
            (find_tail_calls): Add DIAG_MUSTTAIL argument, propagate it down to
            maybe_error_musttail, suitable_for_tail_opt_p,
            suitable_for_tail_call_opt_p and find_tail_calls calls.
            (tree_optimize_tail_calls_1): Add DIAG_MUSTTAIL argument, propagate
            it down to find_tail_calls and if set, clear cfun->has_musttail flag
            at the end.  Rename OPT_MUSTCALL argument to OPT_MUSTTAIL.
            (execute_tail_calls): Pass true to DIAG_MUSTTAIL
            tree_optimize_tail_calls_1 argument.
            (pass_tail_recursion::execute): Pass false to DIAG_MUSTTAIL
            tree_optimize_tail_calls_1 argument.
            (pass_musttail::gate): Don't test flag_optimize_sibling_calls.
            (pass_musttail::execute): Pass true to DIAG_MUSTTAIL
            tree_optimize_tail_calls_1 argument.
    
            * g++.dg/torture/musttail1.C: New test.
            * g++.dg/opt/musttail2.C: New test.

Diff:
---
 gcc/testsuite/g++.dg/opt/musttail2.C     | 14 +++++
 gcc/testsuite/g++.dg/torture/musttail1.C | 15 +++++
 gcc/tree-tailcall.cc                     | 97 +++++++++++++++++++-------------
 3 files changed, 87 insertions(+), 39 deletions(-)

diff --git a/gcc/testsuite/g++.dg/opt/musttail2.C 
b/gcc/testsuite/g++.dg/opt/musttail2.C
new file mode 100644
index 000000000000..ee55c1a18df1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/musttail2.C
@@ -0,0 +1,14 @@
+// PR ipa/119376
+// { dg-do compile { target musttail } }
+// { dg-options "-O2 -fno-early-inlining -fdump-tree-optimized" }
+// { dg-final { scan-tree-dump-times "  \[^\n\r]* = foo \\\(\[^\n\r]*\\\); 
\\\[tail call\\\] \\\[must tail call\\\]" 1 "optimized" } }
+
+struct S { S () {} };
+char *foo (S);
+
+char *
+bar (S)
+{
+  S t;
+  [[clang::musttail]] return foo (t);
+}
diff --git a/gcc/testsuite/g++.dg/torture/musttail1.C 
b/gcc/testsuite/g++.dg/torture/musttail1.C
new file mode 100644
index 000000000000..12012ad22074
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/musttail1.C
@@ -0,0 +1,15 @@
+// PR ipa/119376
+// { dg-do compile { target musttail } }
+// { dg-additional-options "-ffat-lto-objects -fdump-tree-optimized" }
+/* { dg-final { scan-tree-dump-times "  \[^\n\r]* = foo \\\(\[^\n\r]*\\\); 
\\\[tail call\\\] \\\[must tail call\\\]" 1 "optimized" } } */
+
+struct S { int s; };
+int foo (int);
+
+int
+bar (int a)
+{
+  S b = {a};
+  b.s++;
+  [[gnu::musttail]] return foo (a);
+}
diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc
index f97df31eb3cf..3efbe9042f20 100644
--- a/gcc/tree-tailcall.cc
+++ b/gcc/tree-tailcall.cc
@@ -139,18 +139,18 @@ static tree m_acc, a_acc;
 
 static bitmap tailr_arg_needs_copy;
 
-static void maybe_error_musttail (gcall *call, const char *err);
+static void maybe_error_musttail (gcall *call, const char *err, bool);
 
 /* Returns false when the function is not suitable for tail call optimization
    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 (gcall *call)
+suitable_for_tail_opt_p (gcall *call, bool diag_musttail)
 {
   if (cfun->stdarg)
     {
-      maybe_error_musttail (call, _("caller uses stdargs"));
+      maybe_error_musttail (call, _("caller uses stdargs"), diag_musttail);
       return false;
     }
 
@@ -163,7 +163,7 @@ suitable_for_tail_opt_p (gcall *call)
    tail call discovery happen. CALL is call to report error for.  */
 
 static bool
-suitable_for_tail_call_opt_p (gcall *call)
+suitable_for_tail_call_opt_p (gcall *call, bool diag_musttail)
 {
   tree param;
 
@@ -171,7 +171,7 @@ suitable_for_tail_call_opt_p (gcall *call)
      sibling call optimizations, but not tail recursion.  */
   if (cfun->calls_alloca)
     {
-      maybe_error_musttail (call, _("caller uses alloca"));
+      maybe_error_musttail (call, _("caller uses alloca"), diag_musttail);
       return false;
     }
 
@@ -181,7 +181,8 @@ suitable_for_tail_call_opt_p (gcall *call)
   if (targetm_common.except_unwind_info (&global_options) == UI_SJLJ
       && current_function_has_exception_handlers ())
     {
-      maybe_error_musttail (call, _("caller uses sjlj exceptions"));
+      maybe_error_musttail (call, _("caller uses sjlj exceptions"),
+                           diag_musttail);
       return false;
     }
 
@@ -190,7 +191,7 @@ suitable_for_tail_call_opt_p (gcall *call)
      properly in the CFG so that this needn't be special cased.  */
   if (cfun->calls_setjmp)
     {
-      maybe_error_musttail (call, _("caller uses setjmp"));
+      maybe_error_musttail (call, _("caller uses setjmp"), diag_musttail);
       return false;
     }
 
@@ -198,7 +199,8 @@ suitable_for_tail_call_opt_p (gcall *call)
      that call __builtin_eh_return.  */
   if (cfun->calls_eh_return)
     {
-      maybe_error_musttail (call, _("caller uses __builtin_eh_return"));
+      maybe_error_musttail (call, _("caller uses __builtin_eh_return"),
+                           diag_musttail);
       return false;
     }
 
@@ -209,7 +211,8 @@ suitable_for_tail_call_opt_p (gcall *call)
        param = DECL_CHAIN (param))
     if (TREE_ADDRESSABLE (param))
       {
-       maybe_error_musttail (call, _("address of caller arguments taken"));
+       maybe_error_musttail (call, _("address of caller arguments taken"),
+                             diag_musttail);
        return false;
       }
 
@@ -436,9 +439,9 @@ propagate_through_phis (tree var, edge e)
    errors.  */
 
 static void
-maybe_error_musttail (gcall *call, const char *err)
+maybe_error_musttail (gcall *call, const char *err, bool diag_musttail)
 {
-  if (gimple_call_must_tail_p (call))
+  if (gimple_call_must_tail_p (call) && diag_musttail)
     {
       error_at (call->location, "cannot tail-call: %s", err);
       /* Avoid another error. ??? If there are multiple reasons why tail
@@ -461,12 +464,13 @@ static live_vars_map *live_vars;
 static vec<bitmap_head> 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.
-   Update OPT_TAILCALLS as output parameter.  */
+   added to the start of RET.  When ONLY_MUSTTAIL is set only handle musttail.
+   Update OPT_TAILCALLS as output parameter.  If DIAG_MUSTTAIL, diagnose
+   failures for musttail calls.  */
 
 static void
 find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail,
-                bool &opt_tailcalls)
+                bool &opt_tailcalls, bool diag_musttail)
 {
   tree ass_var = NULL_TREE, ret_var, func, param;
   gimple *stmt;
@@ -524,7 +528,7 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
            {
              maybe_error_musttail (call,
                                    _("memory reference or volatile after "
-                                     "call"));
+                                     "call"), diag_musttail);
              return;
            }
          ass_var = gimple_call_lhs (call);
@@ -561,15 +565,16 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
       edge_iterator ei;
       /* Recurse to the predecessors.  */
       FOR_EACH_EDGE (e, ei, bb->preds)
-       find_tail_calls (e->src, ret, only_musttail, opt_tailcalls);
+       find_tail_calls (e->src, ret, only_musttail, opt_tailcalls,
+                        diag_musttail);
 
       return;
     }
 
-  if (!suitable_for_tail_opt_p (call))
+  if (!suitable_for_tail_opt_p (call, diag_musttail))
     return;
 
-  if (!suitable_for_tail_call_opt_p (call))
+  if (!suitable_for_tail_call_opt_p (call, diag_musttail))
     opt_tailcalls = false;
 
   /* If the LHS of our call is not just a simple register or local
@@ -587,13 +592,13 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
       && !is_gimple_reg (ass_var)
       && !auto_var_in_fn_p (ass_var, cfun->decl))
     {
-      maybe_error_musttail (call, _("return value in memory"));
+      maybe_error_musttail (call, _("return value in memory"), diag_musttail);
       return;
     }
 
   if (cfun->calls_setjmp)
     {
-      maybe_error_musttail (call, _("caller uses setjmp"));
+      maybe_error_musttail (call, _("caller uses setjmp"), diag_musttail);
       return;
     }
 
@@ -605,9 +610,10 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
     if (stmt == last_stmt)
       maybe_error_musttail (call,
                            _("call may throw exception that does not "
-                             "propagate"));
+                             "propagate"), diag_musttail);
     else
-      maybe_error_musttail (call, _("code between call and return"));
+      maybe_error_musttail (call, _("code between call and return"),
+                           diag_musttail);
     return;
   }
 
@@ -639,7 +645,8 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
       && may_be_aliased (result_decl)
       && ref_maybe_used_by_stmt_p (call, result_decl, false))
     {
-      maybe_error_musttail (call, _("return value used after call"));
+      maybe_error_musttail (call, _("return value used after call"),
+                           diag_musttail);
       return;
     }
 
@@ -723,7 +730,8 @@ 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"));
+                                   _("call invocation refers to locals"),
+                                   diag_musttail);
              return;
            }
          else
@@ -733,7 +741,8 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
                {
                  BITMAP_FREE (local_live_vars);
                  maybe_error_musttail (call,
-                                       _("call invocation refers to locals"));
+                                       _("call invocation refers to locals"),
+                                       diag_musttail);
                  return;
                }
            }
@@ -780,7 +789,8 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
 
       if (gimple_code (stmt) != GIMPLE_ASSIGN)
        {
-         maybe_error_musttail (call, _("unhandled code after call"));
+         maybe_error_musttail (call, _("unhandled code after call"),
+                               diag_musttail);
          return;
        }
 
@@ -789,7 +799,8 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
                                    &tmp_m, &tmp_a, &ass_var, to_move_defs);
       if (ret == FAIL || (ret == TRY_MOVE && !tail_recursion))
        {
-         maybe_error_musttail (call, _("return value changed after call"));
+         maybe_error_musttail (call, _("return value changed after call"),
+                               diag_musttail);
          return;
        }
       else if (ret == TRY_MOVE)
@@ -864,7 +875,8 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
       if (!ok)
        {
          maybe_error_musttail (call,
-                               _("call and return value are different"));
+                               _("call and return value are different"),
+                               diag_musttail);
          return;
        }
     }
@@ -874,7 +886,8 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
   if (!tail_recursion && (m || a))
     {
       maybe_error_musttail (call,
-                           _("operations after non tail recursive call"));
+                           _("operations after non tail recursive call"),
+                           diag_musttail);
       return;
     }
 
@@ -883,7 +896,7 @@ find_tail_calls (basic_block bb, struct tailcall **ret, 
bool only_musttail,
     {
       maybe_error_musttail (call,
                            _("tail recursion with pointers can only use "
-                             "additions"));
+                             "additions"), diag_musttail);
       return;
     }
 
@@ -1258,11 +1271,13 @@ create_tailcall_accumulator (const char *label, 
basic_block bb, tree init)
 }
 
 /* Optimizes tail calls in the function, turning the tail recursion
-   into iteration. When ONLY_MUSTCALL is true only optimize mustcall
-   marked calls.  */
+   into iteration.  When ONLY_MUSTTAIL is true only optimize musttail
+   marked calls.  When DIAG_MUSTTAIL, diagnose if musttail calls can't
+   be tail call optimized.  */
 
 static unsigned int
-tree_optimize_tail_calls_1 (bool opt_tailcalls, bool only_mustcall)
+tree_optimize_tail_calls_1 (bool opt_tailcalls, bool only_musttail,
+                           bool diag_musttail)
 {
   edge e;
   bool phis_constructed = false;
@@ -1277,7 +1292,8 @@ tree_optimize_tail_calls_1 (bool opt_tailcalls, bool 
only_mustcall)
       /* Only traverse the normal exits, i.e. those that end with return
         statement.  */
       if (safe_is_a <greturn *> (*gsi_last_bb (e->src)))
-       find_tail_calls (e->src, &tailcalls, only_mustcall, opt_tailcalls);
+       find_tail_calls (e->src, &tailcalls, only_musttail, opt_tailcalls,
+                        diag_musttail);
     }
 
   if (live_vars)
@@ -1374,6 +1390,9 @@ tree_optimize_tail_calls_1 (bool opt_tailcalls, bool 
only_mustcall)
   if (tailr_arg_needs_copy)
     BITMAP_FREE (tailr_arg_needs_copy);
 
+  if (diag_musttail)
+    cfun->has_musttail = false;
+
   if (changed)
     return TODO_cleanup_cfg | TODO_update_ssa_only_virtuals;
   return 0;
@@ -1388,7 +1407,7 @@ gate_tail_calls (void)
 static unsigned int
 execute_tail_calls (void)
 {
-  return tree_optimize_tail_calls_1 (true, false);
+  return tree_optimize_tail_calls_1 (true, false, true);
 }
 
 namespace {
@@ -1421,7 +1440,7 @@ public:
   bool gate (function *) final override { return gate_tail_calls (); }
   unsigned int execute (function *) final override
     {
-      return tree_optimize_tail_calls_1 (false, false);
+      return tree_optimize_tail_calls_1 (false, false, false);
     }
 
 }; // class pass_tail_recursion
@@ -1497,15 +1516,15 @@ public:
 
   /* opt_pass methods: */
   /* This pass is only used when the other tail call pass
-     doesn't run to make [[musttail]] still work. But only
+     doesn't run to make [[musttail]] still work.  But only
      run it when there is actually a musttail in the function.  */
   bool gate (function *f) final override
   {
-    return !flag_optimize_sibling_calls && f->has_musttail;
+    return f->has_musttail;
   }
   unsigned int execute (function *) final override
   {
-    return tree_optimize_tail_calls_1 (true, true);
+    return tree_optimize_tail_calls_1 (true, true, true);
   }
 
 }; // class pass_musttail

Reply via email to