public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Updated musttail patches
@ 2024-02-02  9:09 Andi Kleen
  2024-02-02  9:09 ` [PATCH v4 1/5] Improve must tail in RTL backend Andi Kleen
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Andi Kleen @ 2024-02-02  9:09 UTC (permalink / raw)
  To: gcc-patches

This patchkit implements a [[musttail]] attribute for C/C++.

v4:
Addressed all feedback except clang::musttail is still supported
(I don't want to force an #ifdef to most users) and I'm also still
using the lookup/remove_attributes (not clear if anything else
is worth it and it would certainly be more complicated)

Now added Changelogs.
Unified the tests for C/C++
C++ now checks for bogus arguments to gnu::musttail
Don't limit C++ tests to C++23
Added noipa to test cases (probably won't make any difference)
Other minor changes



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 1/5] Improve must tail in RTL backend
  2024-02-02  9:09 Updated musttail patches Andi Kleen
@ 2024-02-02  9:09 ` Andi Kleen
  2024-02-02  9:09 ` [PATCH v4 2/5] C++: Support clang compatible [[musttail]] (PR83324) Andi Kleen
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Andi Kleen @ 2024-02-02  9:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

- Give error messages for all causes of non sibling call generation
- Don't override choices of other non sibling call checks with
must tail. This causes ICEs. The must tail attribute now only
overrides flag_optimize_sibling_calls locally.
- Error out when tree-tailcall failed to mark a must-tail call
sibcall. In this case it doesn't know the true reason and only gives
a vague message (this could be improved, but it's already useful without
that) tree-tailcall usually fails without optimization, so must
adjust the existing must-tail plugin test to specify -O2.

	PR83324

gcc/ChangeLog:

	* calls.cc (expand_call): Fix mustcall implementation.

gcc/testsuite/ChangeLog:

	* gcc.dg/plugin/must-tail-call-1.c: Adjust.
---
 gcc/calls.cc                                  | 30 ++++++++++++-------
 .../gcc.dg/plugin/must-tail-call-1.c          |  1 +
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/gcc/calls.cc b/gcc/calls.cc
index 01f447347437..60a43f5d36a8 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -2650,7 +2650,9 @@ expand_call (tree exp, rtx target, int ignore)
   /* The type of the function being called.  */
   tree fntype;
   bool try_tail_call = CALL_EXPR_TAILCALL (exp);
-  bool must_tail_call = CALL_EXPR_MUST_TAIL_CALL (exp);
+  /* tree-tailcall decided not to do tail calls. Error for the musttail case.  */
+  if (!try_tail_call)
+      maybe_complain_about_tail_call (exp, "other reasons");
   int pass;
 
   /* Register in which non-BLKmode value will be returned,
@@ -3021,10 +3023,22 @@ expand_call (tree exp, rtx target, int ignore)
      pushed these optimizations into -O2.  Don't try if we're already
      expanding a call, as that means we're an argument.  Don't try if
      there's cleanups, as we know there's code to follow the call.  */
-  if (currently_expanding_call++ != 0
-      || (!flag_optimize_sibling_calls && !CALL_FROM_THUNK_P (exp))
-      || args_size.var
-      || dbg_cnt (tail_call) == false)
+  if (currently_expanding_call++ != 0)
+    {
+      maybe_complain_about_tail_call (exp, "inside another call");
+      try_tail_call = 0;
+    }
+  if (!flag_optimize_sibling_calls
+	&& !CALL_FROM_THUNK_P (exp)
+	&& !CALL_EXPR_MUST_TAIL_CALL (exp))
+    try_tail_call = 0;
+  if (args_size.var)
+    {
+      /* ??? correct message?  */
+      maybe_complain_about_tail_call (exp, "stack space needed");
+      try_tail_call = 0;
+    }
+  if (dbg_cnt (tail_call) == false)
     try_tail_call = 0;
 
   /* Workaround buggy C/C++ wrappers around Fortran routines with
@@ -3045,15 +3059,11 @@ expand_call (tree exp, rtx target, int ignore)
 	    if (MEM_P (*iter))
 	      {
 		try_tail_call = 0;
+		maybe_complain_about_tail_call (exp, "hidden string length argument");
 		break;
 	      }
 	}
 
-  /* If the user has marked the function as requiring tail-call
-     optimization, attempt it.  */
-  if (must_tail_call)
-    try_tail_call = 1;
-
   /*  Rest of purposes for tail call optimizations to fail.  */
   if (try_tail_call)
     try_tail_call = can_implement_as_sibling_call_p (exp,
diff --git a/gcc/testsuite/gcc.dg/plugin/must-tail-call-1.c b/gcc/testsuite/gcc.dg/plugin/must-tail-call-1.c
index 3a6d4cceaba7..44af361e2925 100644
--- a/gcc/testsuite/gcc.dg/plugin/must-tail-call-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/must-tail-call-1.c
@@ -1,4 +1,5 @@
 /* { dg-do compile { target tail_call } } */
+/* { dg-options "-O2" } */
 /* { dg-options "-fdelayed-branch" { target sparc*-*-* } } */
 
 extern void abort (void);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 2/5] C++: Support clang compatible [[musttail]] (PR83324)
  2024-02-02  9:09 Updated musttail patches Andi Kleen
  2024-02-02  9:09 ` [PATCH v4 1/5] Improve must tail in RTL backend Andi Kleen
@ 2024-02-02  9:09 ` Andi Kleen
  2024-02-02  9:09 ` [PATCH v4 3/5] C: Implement musttail attribute for returns Andi Kleen
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Andi Kleen @ 2024-02-02  9:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

This patch implements a clang compatible [[musttail]] attribute for
returns.

musttail is useful as an alternative to computed goto for interpreters.
With computed goto the interpreter function usually ends up very big
which causes problems with register allocation and other per function
optimizations not scaling. With musttail the interpreter can be instead
written as a sequence of smaller functions that call each other. To
avoid unbounded stack growth this requires forcing a sibling call, which
this attribute does. It guarantees an error if the call cannot be tail
called which allows the programmer to fix it instead of risking a stack
overflow. Unlike computed goto it is also type-safe.

It turns out that David Malcolm had already implemented middle/backend
support for a musttail attribute back in 2016, but it wasn't exposed
to any frontend other than a special plugin.

This patch adds a [[gnu::musttail]] attribute for C++ that can be added
to return statements. The return statement must be a direct call
(it does not follow dependencies), which is similar to what clang
implements. It then uses the existing must tail infrastructure.

For compatibility it also detects clang::musttail

One problem is that tree-tailcall usually fails when optimization
is disabled, which implies the attribute only really works with
optimization on. But that seems to be a reasonable limitation.

Passes bootstrap and full test

	PR83324

gcc/cp/ChangeLog:

	* cp-tree.h (finish_return_stmt): Add musttail_p.
	(check_return_expr): Dito.
	* parser.cc (cp_parser_statement): Handle [[musttail]].
	(cp_parser_std_attribute): Dito.
	(cp_parser_init_statement): Dito.
	(cp_parser_jump_statement): Dito.
	* semantics.cc (finish_return_stmt): Dito.
	* typeck.cc (check_return_expr): Handle musttail_p flag.
---
 gcc/cp/cp-tree.h    |  4 ++--
 gcc/cp/parser.cc    | 30 ++++++++++++++++++++++++------
 gcc/cp/semantics.cc |  6 +++---
 gcc/cp/typeck.cc    | 20 ++++++++++++++++++--
 4 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f46b448ce0d2..efc542c3ec26 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7763,7 +7763,7 @@ extern void finish_while_stmt			(tree);
 extern tree begin_do_stmt			(void);
 extern void finish_do_body			(tree);
 extern void finish_do_stmt		(tree, tree, bool, tree, bool);
-extern tree finish_return_stmt			(tree);
+extern tree finish_return_stmt			(tree, bool = false);
 extern tree begin_for_scope			(tree *);
 extern tree begin_for_stmt			(tree, tree);
 extern void finish_init_stmt			(tree);
@@ -8275,7 +8275,7 @@ extern tree composite_pointer_type		(const op_location_t &,
 						 tsubst_flags_t);
 extern tree merge_types				(tree, tree);
 extern tree strip_array_domain			(tree);
-extern tree check_return_expr			(tree, bool *, bool *);
+extern tree check_return_expr			(tree, bool *, bool *, bool);
 extern tree spaceship_type			(tree, tsubst_flags_t = tf_warning_or_error);
 extern tree genericize_spaceship		(location_t, tree, tree, tree);
 extern tree cp_build_binary_op                  (const op_location_t &,
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 3748ccd49ff3..f56500df3725 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -2462,7 +2462,7 @@ static tree cp_parser_perform_range_for_lookup
 static tree cp_parser_range_for_member_function
   (tree, tree);
 static tree cp_parser_jump_statement
-  (cp_parser *);
+  (cp_parser *, bool = false);
 static void cp_parser_declaration_statement
   (cp_parser *);
 
@@ -12719,9 +12719,27 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 						     NULL_TREE, false);
 	  break;
 
+	case RID_RETURN:
+	  {
+	    bool musttail_p = false;
+	    std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
+	    if (lookup_attribute ("gnu", "musttail", std_attrs))
+	      {
+		musttail_p = true;
+		std_attrs = remove_attribute ("gnu", "musttail", std_attrs);
+	      }
+	    // support this for compatibility
+	    if (lookup_attribute ("clang", "musttail", std_attrs))
+	      {
+		musttail_p = true;
+		std_attrs = remove_attribute ("clang", "musttail", std_attrs);
+	      }
+	    statement = cp_parser_jump_statement (parser, musttail_p);
+	  }
+	  break;
+
 	case RID_BREAK:
 	case RID_CONTINUE:
-	case RID_RETURN:
 	case RID_CO_RETURN:
 	case RID_GOTO:
 	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
@@ -14767,7 +14785,7 @@ cp_parser_init_statement (cp_parser *parser, tree *decl)
   return false;
 }
 
-/* Parse a jump-statement.
+/* Parse a jump-statement.  MUSTTAIL_P indicates a musttail attribute.
 
    jump-statement:
      break ;
@@ -14785,7 +14803,7 @@ cp_parser_init_statement (cp_parser *parser, tree *decl)
    Returns the new BREAK_STMT, CONTINUE_STMT, RETURN_EXPR, or GOTO_EXPR.  */
 
 static tree
-cp_parser_jump_statement (cp_parser* parser)
+cp_parser_jump_statement (cp_parser* parser, bool musttail_p)
 {
   tree statement = error_mark_node;
   cp_token *token;
@@ -14869,7 +14887,7 @@ cp_parser_jump_statement (cp_parser* parser)
 	else if (FNDECL_USED_AUTO (current_function_decl) && in_discarded_stmt)
 	  /* Don't deduce from a discarded return statement.  */;
 	else
-	  statement = finish_return_stmt (expr);
+	  statement = finish_return_stmt (expr, musttail_p);
 	/* Look for the final `;'.  */
 	cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
       }
@@ -30041,7 +30059,7 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
     /* Maybe we don't expect to see any arguments for this attribute.  */
     const attribute_spec *as
       = lookup_attribute_spec (TREE_PURPOSE (attribute));
-    if (as && as->max_length == 0)
+    if ((as && as->max_length == 0) || is_attribute_p ("musttail", attr_id))
       {
 	error_at (token->location, "%qE attribute does not take any arguments",
 		  attr_id);
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 3299e2704465..a277f70ea0fd 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -1324,16 +1324,16 @@ finish_do_stmt (tree cond, tree do_stmt, bool ivdep, tree unroll,
 }
 
 /* Finish a return-statement.  The EXPRESSION returned, if any, is as
-   indicated.  */
+   indicated.  MUSTTAIL_P indicates a mustcall attribute.  */
 
 tree
-finish_return_stmt (tree expr)
+finish_return_stmt (tree expr, bool musttail_p)
 {
   tree r;
   bool no_warning;
   bool dangling;
 
-  expr = check_return_expr (expr, &no_warning, &dangling);
+  expr = check_return_expr (expr, &no_warning, &dangling, musttail_p);
 
   if (error_operand_p (expr)
       || (flag_openmp && !check_omp_return ()))
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 4937022ff207..7ea292e16851 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11030,10 +11030,12 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
    the DECL_RESULT for the function.  Set *NO_WARNING to true if
    code reaches end of non-void function warning shouldn't be issued
    on this RETURN_EXPR.  Set *DANGLING to true if code returns the
-   address of a local variable.  */
+   address of a local variable.  MUSTTAIL_P indicates a musttail
+   return.  */
 
 tree
-check_return_expr (tree retval, bool *no_warning, bool *dangling)
+check_return_expr (tree retval, bool *no_warning, bool *dangling,
+		   bool musttail_p)
 {
   tree result;
   /* The type actually returned by the function.  */
@@ -11047,6 +11049,20 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
   *no_warning = false;
   *dangling = false;
 
+  if (musttail_p)
+    {
+      tree t = retval;
+      if (TREE_CODE (t) == TARGET_EXPR)
+	t = TARGET_EXPR_INITIAL (retval);
+      if (TREE_CODE (t) != CALL_EXPR)
+	{
+	  error_at (loc, "cannot tail-call: return value must be a call");
+	  return error_mark_node;
+	}
+      else
+	CALL_EXPR_MUST_TAIL_CALL (t) = 1;
+    }
+
   /* A `volatile' function is one that isn't supposed to return, ever.
      (This is a G++ extension, used to get better code for functions
      that call the `volatile' function.)  */
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 3/5] C: Implement musttail attribute for returns
  2024-02-02  9:09 Updated musttail patches Andi Kleen
  2024-02-02  9:09 ` [PATCH v4 1/5] Improve must tail in RTL backend Andi Kleen
  2024-02-02  9:09 ` [PATCH v4 2/5] C++: Support clang compatible [[musttail]] (PR83324) Andi Kleen
@ 2024-02-02  9:09 ` Andi Kleen
  2024-02-02  9:09 ` [PATCH v4 4/5] Add tests for C/C++ musttail attributes Andi Kleen
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Andi Kleen @ 2024-02-02  9:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

Implement a C23 clang compatible musttail attribute similar to the earlier
C++ implementation in the C parser.

	PR83324

gcc/c/ChangeLog:

	* c-parser.cc (struct attr_state): Define with musttail_p.
	(c_parser_statement_after_labels): Handle [[musttail]]
	(c_parser_std_attribute): Dito.
	(c_parser_handle_musttail): Dito.
	(c_parser_compound_statement_nostart): Dito.
	(c_parser_all_labels): Dito.
	(c_parser_statement): Dito.
	* c-tree.h (c_finish_return): Add musttail_p flag.
	* c-typeck.cc (c_finish_return): Handle musttail_p flag.
---
 gcc/c/c-parser.cc | 61 +++++++++++++++++++++++++++++++++++++----------
 gcc/c/c-tree.h    |  2 +-
 gcc/c/c-typeck.cc | 15 ++++++++++--
 3 files changed, 63 insertions(+), 15 deletions(-)

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index c31349dae2ff..76931931e270 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -1616,6 +1616,11 @@ struct omp_for_parse_data {
   bool fail : 1;
 };
 
+struct attr_state
+{
+  bool musttail_p; // parsed a musttail for return
+};
+
 static bool c_parser_nth_token_starts_std_attributes (c_parser *,
 						      unsigned int);
 static tree c_parser_std_attribute_specifier_sequence (c_parser *);
@@ -1660,7 +1665,7 @@ static location_t c_parser_compound_statement_nostart (c_parser *);
 static void c_parser_label (c_parser *, tree);
 static void c_parser_statement (c_parser *, bool *, location_t * = NULL);
 static void c_parser_statement_after_labels (c_parser *, bool *,
-					     vec<tree> * = NULL);
+					     vec<tree> * = NULL, attr_state = {});
 static tree c_parser_c99_block_statement (c_parser *, bool *,
 					  location_t * = NULL);
 static void c_parser_if_statement (c_parser *, bool *, vec<tree> *);
@@ -5757,6 +5762,8 @@ c_parser_std_attribute (c_parser *parser, bool for_tm,
 	}
       goto out;
     }
+  else if (is_attribute_p ("musttail", name))
+    error ("%<musttail%> attribute has arguments");
   {
     location_t open_loc = c_parser_peek_token (parser)->location;
     matching_parens parens;
@@ -6943,6 +6950,28 @@ c_parser_handle_directive_omp_attributes (tree &attrs,
     }
 }
 
+/* Check if STD_ATTR contains a musttail attribute and handle it
+   PARSER is the parser and A is the output attr_state.  */
+
+static tree
+c_parser_handle_musttail (c_parser *parser, tree std_attrs, attr_state &a)
+{
+  if (c_parser_next_token_is_keyword (parser, RID_RETURN))
+    {
+      if (lookup_attribute ("gnu", "musttail", std_attrs))
+	{
+	  std_attrs = remove_attribute ("gnu", "musttail", std_attrs);
+	  a.musttail_p = true;
+	}
+      if (lookup_attribute ("clang", "musttail", std_attrs))
+	{
+	  std_attrs = remove_attribute ("clang", "musttail", std_attrs);
+	  a.musttail_p = true;
+	}
+    }
+  return std_attrs;
+}
+
 /* Parse a compound statement except for the opening brace.  This is
    used for parsing both compound statements and statement expressions
    (which follow different paths to handling the opening).  */
@@ -6959,6 +6988,7 @@ c_parser_compound_statement_nostart (c_parser *parser)
   bool in_omp_loop_block
     = omp_for_parse_state ? omp_for_parse_state->want_nested_loop : false;
   tree sl = NULL_TREE;
+  attr_state a = {};
 
   if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
     {
@@ -7097,7 +7127,10 @@ c_parser_compound_statement_nostart (c_parser *parser)
 	= c_parser_nth_token_starts_std_attributes (parser, 1);
       tree std_attrs = NULL_TREE;
       if (have_std_attrs)
-	std_attrs = c_parser_std_attribute_specifier_sequence (parser);
+	{
+	  std_attrs = c_parser_std_attribute_specifier_sequence (parser);
+	  std_attrs = c_parser_handle_musttail (parser, std_attrs, a);
+	}
       if (c_parser_next_token_is_keyword (parser, RID_CASE)
 	  || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
 	  || (c_parser_next_token_is (parser, CPP_NAME)
@@ -7245,7 +7278,7 @@ c_parser_compound_statement_nostart (c_parser *parser)
 	  last_stmt = true;
 	  mark_valid_location_for_stdc_pragma (false);
 	  if (!omp_for_parse_state)
-	    c_parser_statement_after_labels (parser, NULL);
+	    c_parser_statement_after_labels (parser, NULL, NULL, a);
 	  else
 	    {
 	      /* In canonical loop nest form, nested loops can only appear
@@ -7287,15 +7320,18 @@ c_parser_compound_statement_nostart (c_parser *parser)
 /* Parse all consecutive labels, possibly preceded by standard
    attributes.  In this context, a statement is required, not a
    declaration, so attributes must be followed by a statement that is
-   not just a semicolon.  */
+   not just a semicolon.  Returns an attr_state.  */
 
-static void
+static attr_state
 c_parser_all_labels (c_parser *parser)
 {
+  attr_state a = {};
   bool have_std_attrs;
   tree std_attrs = NULL;
   if ((have_std_attrs = c_parser_nth_token_starts_std_attributes (parser, 1)))
-    std_attrs = c_parser_std_attribute_specifier_sequence (parser);
+    std_attrs = c_parser_handle_musttail (parser,
+		    c_parser_std_attribute_specifier_sequence (parser), a);
+
   while (c_parser_next_token_is_keyword (parser, RID_CASE)
 	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
 	 || (c_parser_next_token_is (parser, CPP_NAME)
@@ -7317,6 +7353,7 @@ c_parser_all_labels (c_parser *parser)
     }
   else if (have_std_attrs && c_parser_next_token_is (parser, CPP_SEMICOLON))
     c_parser_error (parser, "expected statement");
+  return a;
 }
 
 /* Parse a label (C90 6.6.1, C99 6.8.1, C11 6.8.1).
@@ -7560,11 +7597,11 @@ c_parser_label (c_parser *parser, tree std_attrs)
 static void
 c_parser_statement (c_parser *parser, bool *if_p, location_t *loc_after_labels)
 {
-  c_parser_all_labels (parser);
+  attr_state a = c_parser_all_labels (parser);
   if (loc_after_labels)
     *loc_after_labels = c_parser_peek_token (parser)->location;
   parser->omp_attrs_forbidden_p = false;
-  c_parser_statement_after_labels (parser, if_p, NULL);
+  c_parser_statement_after_labels (parser, if_p, NULL, a);
 }
 
 /* Parse a statement, other than a labeled statement.  CHAIN is a vector
@@ -7573,11 +7610,11 @@ c_parser_statement (c_parser *parser, bool *if_p, location_t *loc_after_labels)
 
    IF_P is used to track whether there's a (possibly labeled) if statement
    which is not enclosed in braces and has an else clause.  This is used to
-   implement -Wparentheses.  */
+   implement -Wparentheses. A has an earlier parsed attribute state.  */
 
 static void
 c_parser_statement_after_labels (c_parser *parser, bool *if_p,
-				 vec<tree> *chain)
+				 vec<tree> *chain, attr_state a)
 {
   location_t loc = c_parser_peek_token (parser)->location;
   tree stmt = NULL_TREE;
@@ -7645,7 +7682,7 @@ c_parser_statement_after_labels (c_parser *parser, bool *if_p,
 	  c_parser_consume_token (parser);
 	  if (c_parser_next_token_is (parser, CPP_SEMICOLON))
 	    {
-	      stmt = c_finish_return (loc, NULL_TREE, NULL_TREE);
+	      stmt = c_finish_return (loc, NULL_TREE, NULL_TREE, a.musttail_p);
 	      c_parser_consume_token (parser);
 	    }
 	  else
@@ -7654,7 +7691,7 @@ c_parser_statement_after_labels (c_parser *parser, bool *if_p,
 	      struct c_expr expr = c_parser_expression_conv (parser);
 	      mark_exp_read (expr.value);
 	      stmt = c_finish_return (EXPR_LOC_OR_LOC (expr.value, xloc),
-				      expr.value, expr.original_type);
+				      expr.value, expr.original_type, a.musttail_p);
 	      goto expect_semicolon;
 	    }
 	  break;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 1fba9c8dae76..efea9e913a50 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -825,7 +825,7 @@ extern tree c_begin_stmt_expr (void);
 extern tree c_finish_stmt_expr (location_t, tree);
 extern tree c_process_expr_stmt (location_t, tree);
 extern tree c_finish_expr_stmt (location_t, tree);
-extern tree c_finish_return (location_t, tree, tree);
+extern tree c_finish_return (location_t, tree, tree, bool = false);
 extern tree c_finish_bc_stmt (location_t, tree, bool);
 extern tree c_finish_goto_label (location_t, tree);
 extern tree c_finish_goto_ptr (location_t, c_expr val);
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 3b519c48ae0a..695f1d519770 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -11443,10 +11443,10 @@ c_finish_goto_ptr (location_t loc, c_expr val)
    to return, or a null pointer for `return;' with no value.  LOC is
    the location of the return statement, or the location of the expression,
    if the statement has any.  If ORIGTYPE is not NULL_TREE, it
-   is the original type of RETVAL.  */
+   is the original type of RETVAL.  MUSTTAIL_P indicates a musttail attribute.  */
 
 tree
-c_finish_return (location_t loc, tree retval, tree origtype)
+c_finish_return (location_t loc, tree retval, tree origtype, bool musttail_p)
 {
   tree valtype = TREE_TYPE (TREE_TYPE (current_function_decl)), ret_stmt;
   bool no_warning = false;
@@ -11460,6 +11460,17 @@ c_finish_return (location_t loc, tree retval, tree origtype)
     warning_at (xloc, 0,
 		"function declared %<noreturn%> has a %<return%> statement");
 
+  if (retval && musttail_p)
+    {
+      tree t = retval;
+      if (TREE_CODE (t) == TARGET_EXPR)
+	t = TARGET_EXPR_INITIAL (t);
+      if (TREE_CODE (t) != CALL_EXPR)
+	error_at (xloc, "cannot tail-call: return value must be call");
+      else
+	CALL_EXPR_MUST_TAIL_CALL (t) = 1;
+    }
+
   if (retval)
     {
       tree semantic_type = NULL_TREE;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 4/5] Add tests for C/C++ musttail attributes
  2024-02-02  9:09 Updated musttail patches Andi Kleen
                   ` (2 preceding siblings ...)
  2024-02-02  9:09 ` [PATCH v4 3/5] C: Implement musttail attribute for returns Andi Kleen
@ 2024-02-02  9:09 ` Andi Kleen
  2024-02-02 11:01   ` Prathamesh Kulkarni
  2024-02-02  9:09 ` [PATCH v4 5/5] Add documentation for musttail attribute Andi Kleen
  2024-02-02 13:18 ` Updated musttail patches Joseph Myers
  5 siblings, 1 reply; 12+ messages in thread
From: Andi Kleen @ 2024-02-02  9:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

Mostly adopted from the existing C musttail plugin tests.

gcc/testsuite/ChangeLog:

	* c-c++-common/musttail1.c: New test.
	* c-c++-common/musttail2.c: New test.
	* c-c++-common/musttail3.c: New test.
	* c-c++-common/musttail4.c: New test.
	* c-c++-common/musttail5.c: New test.
---
 gcc/testsuite/c-c++-common/musttail1.c | 15 ++++++++++++
 gcc/testsuite/c-c++-common/musttail2.c | 34 ++++++++++++++++++++++++++
 gcc/testsuite/c-c++-common/musttail3.c | 29 ++++++++++++++++++++++
 gcc/testsuite/c-c++-common/musttail4.c | 17 +++++++++++++
 gcc/testsuite/c-c++-common/musttail5.c | 25 +++++++++++++++++++
 5 files changed, 120 insertions(+)
 create mode 100644 gcc/testsuite/c-c++-common/musttail1.c
 create mode 100644 gcc/testsuite/c-c++-common/musttail2.c
 create mode 100644 gcc/testsuite/c-c++-common/musttail3.c
 create mode 100644 gcc/testsuite/c-c++-common/musttail4.c
 create mode 100644 gcc/testsuite/c-c++-common/musttail5.c

diff --git a/gcc/testsuite/c-c++-common/musttail1.c b/gcc/testsuite/c-c++-common/musttail1.c
new file mode 100644
index 000000000000..ac92f9f74616
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/musttail1.c
@@ -0,0 +1,15 @@
+/* { dg-do compile { target { tail_call && { c || c++11 } } } } */
+/* { dg-options "-O2" } */
+/* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
+
+int __attribute__((noinline,noclone,noipa))
+callee (int i)
+{
+  return i * i;
+}
+
+int __attribute__((noinline,noclone,noipa))
+caller (int i)
+{
+  [[gnu::musttail]] return callee (i + 1);
+}
diff --git a/gcc/testsuite/c-c++-common/musttail2.c b/gcc/testsuite/c-c++-common/musttail2.c
new file mode 100644
index 000000000000..058329b69cc2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/musttail2.c
@@ -0,0 +1,34 @@
+/* { dg-do compile { target { tail_call && { c || c++11 } } } } */
+
+struct box { char field[256]; int i; };
+
+int __attribute__((noinline,noclone,noipa))
+test_2_callee (int i, struct box b)
+{
+  if (b.field[0])
+    return 5;
+  return i * i;
+}
+
+int __attribute__((noinline,noclone,noipa))
+test_2_caller (int i)
+{
+  struct box b;
+  [[gnu::musttail]] return test_2_callee (i + 1, b); /* { dg-error "cannot tail-call: " } */
+}
+
+extern void setjmp (void);
+void
+test_3 (void)
+{
+  [[gnu::musttail]] return setjmp (); /* { dg-error "cannot tail-call: " } */
+}
+
+typedef void (fn_ptr_t) (void);
+volatile fn_ptr_t fn_ptr;
+
+void
+test_5 (void)
+{
+  [[gnu::musttail]] return fn_ptr (); /* { dg-error "cannot tail-call: " } */
+}
diff --git a/gcc/testsuite/c-c++-common/musttail3.c b/gcc/testsuite/c-c++-common/musttail3.c
new file mode 100644
index 000000000000..ea9589c59ef2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/musttail3.c
@@ -0,0 +1,29 @@
+/* { dg-do compile { target { tail_call && { c || c++11 } } } } */
+
+extern int foo2 (int x, ...);
+
+struct str
+{
+  int a, b;
+};
+
+struct str
+cstruct (int x)
+{
+  if (x < 10)
+    [[clang::musttail]] return cstruct (x + 1);
+  return ((struct str){ x, 0 });
+}
+
+int
+foo (int x)
+{
+  if (x < 10)
+    [[clang::musttail]] return foo2 (x, 29);
+  if (x < 100)
+    {
+      int k = foo (x + 1);
+      [[clang::musttail]] return k;	/* { dg-error "cannot tail-call: " } */
+    }
+  return x;
+}
diff --git a/gcc/testsuite/c-c++-common/musttail4.c b/gcc/testsuite/c-c++-common/musttail4.c
new file mode 100644
index 000000000000..23f4b5e1cd68
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/musttail4.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target { tail_call && { c || c++11 } } } } */
+
+struct box { char field[64]; int i; };
+
+struct box __attribute__((noinline,noclone,noipa))
+returns_struct (int i)
+{
+  struct box b;
+  b.i = i * i;
+  return b;
+}
+
+int __attribute__((noinline,noclone))
+test_1 (int i)
+{
+  [[gnu::musttail]] return returns_struct (i * 5).i; /* { dg-error "cannot tail-call: " } */
+}
diff --git a/gcc/testsuite/c-c++-common/musttail5.c b/gcc/testsuite/c-c++-common/musttail5.c
new file mode 100644
index 000000000000..71f4de40fc6d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/musttail5.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c23" { target c } } */
+/* { dg-options "-std=gnu++11" { target c++ } } */
+
+[[musttail]] int j; /* { dg-warning "attribute" } */
+__attribute__((musttail)) int k; /* { dg-warning "attribute" } */
+
+void foo(void)
+{
+  [[gnu::musttail]] j++; /* { dg-warning "attribute" } */
+  [[gnu::musttail]] if (k > 0) /* { dg-warning "attribute" } */
+    [[gnu::musttail]] k++; /* { dg-warning "attribute" } */
+}
+
+int foo2(int p)
+{
+  [[gnu::musttail(1)]] return foo2(p + 1); /* { dg-error "attribute" } */
+}
+
+int i;
+
+void foo3(void)
+{
+  [[musttail]] i++; /* { dg-warning "attribute" } */
+}
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 5/5] Add documentation for musttail attribute
  2024-02-02  9:09 Updated musttail patches Andi Kleen
                   ` (3 preceding siblings ...)
  2024-02-02  9:09 ` [PATCH v4 4/5] Add tests for C/C++ musttail attributes Andi Kleen
@ 2024-02-02  9:09 ` Andi Kleen
  2024-02-04  4:35   ` Sandra Loosemore
  2024-02-02 13:18 ` Updated musttail patches Joseph Myers
  5 siblings, 1 reply; 12+ messages in thread
From: Andi Kleen @ 2024-02-02  9:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

gcc/ChangeLog:

	* doc/extend.texi: Document [[musttail]]
---
 gcc/doc/extend.texi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 142e41ab8fbf..866f6c4a9fed 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9875,6 +9875,22 @@ foo (int x, int y)
 @code{y} is not actually incremented and the compiler can but does not
 have to optimize it to just @code{return 42 + 42;}.
 
+@cindex @code{musttail} statement attribute
+@item musttail
+
+The @code{gnu::musttail} or @code{clang::musttail} attribute
+can be applied to a return statement that returns the value
+of a call to indicate that the call must be a tail call
+that does not allocate extra stack space.
+
+@smallexample
+[[gnu::musttail]] return foo();
+@end smallexample
+
+If the compiler cannot generate a tail call it will generate
+an error. Tail calls generally require enabling optimization.
+On some targets they may not be supported.
+
 @end table
 
 @node Attribute Syntax
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 4/5] Add tests for C/C++ musttail attributes
  2024-02-02  9:09 ` [PATCH v4 4/5] Add tests for C/C++ musttail attributes Andi Kleen
@ 2024-02-02 11:01   ` Prathamesh Kulkarni
  2024-02-02 14:50     ` Andi Kleen
  0 siblings, 1 reply; 12+ messages in thread
From: Prathamesh Kulkarni @ 2024-02-02 11:01 UTC (permalink / raw)
  To: Andi Kleen; +Cc: gcc-patches

On Fri, 2 Feb 2024 at 14:44, Andi Kleen <ak@linux.intel.com> wrote:
>
> Mostly adopted from the existing C musttail plugin tests.
>
> gcc/testsuite/ChangeLog:
>
>         * c-c++-common/musttail1.c: New test.
>         * c-c++-common/musttail2.c: New test.
>         * c-c++-common/musttail3.c: New test.
>         * c-c++-common/musttail4.c: New test.
>         * c-c++-common/musttail5.c: New test.
> ---
>  gcc/testsuite/c-c++-common/musttail1.c | 15 ++++++++++++
>  gcc/testsuite/c-c++-common/musttail2.c | 34 ++++++++++++++++++++++++++
>  gcc/testsuite/c-c++-common/musttail3.c | 29 ++++++++++++++++++++++
>  gcc/testsuite/c-c++-common/musttail4.c | 17 +++++++++++++
>  gcc/testsuite/c-c++-common/musttail5.c | 25 +++++++++++++++++++
>  5 files changed, 120 insertions(+)
>  create mode 100644 gcc/testsuite/c-c++-common/musttail1.c
>  create mode 100644 gcc/testsuite/c-c++-common/musttail2.c
>  create mode 100644 gcc/testsuite/c-c++-common/musttail3.c
>  create mode 100644 gcc/testsuite/c-c++-common/musttail4.c
>  create mode 100644 gcc/testsuite/c-c++-common/musttail5.c
>
> diff --git a/gcc/testsuite/c-c++-common/musttail1.c b/gcc/testsuite/c-c++-common/musttail1.c
> new file mode 100644
> index 000000000000..ac92f9f74616
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/musttail1.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile { target { tail_call && { c || c++11 } } } } */
> +/* { dg-options "-O2" } */
> +/* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
> +
> +int __attribute__((noinline,noclone,noipa))
> +callee (int i)
Hi Andy,
Sorry, I wasn't clear about this in previous patch -- noipa will
subsume other ipa attributes,
so there's no need to have noinline, noclone along with noipa.
int __attribute__((noipa)) callee(int i) should be sufficient for
disabling IPA optimizations involving callee.

Thanks,
Prathamesh

> +{
> +  return i * i;
> +}
> +
> +int __attribute__((noinline,noclone,noipa))
> +caller (int i)
> +{
> +  [[gnu::musttail]] return callee (i + 1);
> +}
> diff --git a/gcc/testsuite/c-c++-common/musttail2.c b/gcc/testsuite/c-c++-common/musttail2.c
> new file mode 100644
> index 000000000000..058329b69cc2
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/musttail2.c
> @@ -0,0 +1,34 @@
> +/* { dg-do compile { target { tail_call && { c || c++11 } } } } */
> +
> +struct box { char field[256]; int i; };
> +
> +int __attribute__((noinline,noclone,noipa))
> +test_2_callee (int i, struct box b)
> +{
> +  if (b.field[0])
> +    return 5;
> +  return i * i;
> +}
> +
> +int __attribute__((noinline,noclone,noipa))
> +test_2_caller (int i)
> +{
> +  struct box b;
> +  [[gnu::musttail]] return test_2_callee (i + 1, b); /* { dg-error "cannot tail-call: " } */
> +}
> +
> +extern void setjmp (void);
> +void
> +test_3 (void)
> +{
> +  [[gnu::musttail]] return setjmp (); /* { dg-error "cannot tail-call: " } */
> +}
> +
> +typedef void (fn_ptr_t) (void);
> +volatile fn_ptr_t fn_ptr;
> +
> +void
> +test_5 (void)
> +{
> +  [[gnu::musttail]] return fn_ptr (); /* { dg-error "cannot tail-call: " } */
> +}
> diff --git a/gcc/testsuite/c-c++-common/musttail3.c b/gcc/testsuite/c-c++-common/musttail3.c
> new file mode 100644
> index 000000000000..ea9589c59ef2
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/musttail3.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile { target { tail_call && { c || c++11 } } } } */
> +
> +extern int foo2 (int x, ...);
> +
> +struct str
> +{
> +  int a, b;
> +};
> +
> +struct str
> +cstruct (int x)
> +{
> +  if (x < 10)
> +    [[clang::musttail]] return cstruct (x + 1);
> +  return ((struct str){ x, 0 });
> +}
> +
> +int
> +foo (int x)
> +{
> +  if (x < 10)
> +    [[clang::musttail]] return foo2 (x, 29);
> +  if (x < 100)
> +    {
> +      int k = foo (x + 1);
> +      [[clang::musttail]] return k;    /* { dg-error "cannot tail-call: " } */
> +    }
> +  return x;
> +}
> diff --git a/gcc/testsuite/c-c++-common/musttail4.c b/gcc/testsuite/c-c++-common/musttail4.c
> new file mode 100644
> index 000000000000..23f4b5e1cd68
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/musttail4.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile { target { tail_call && { c || c++11 } } } } */
> +
> +struct box { char field[64]; int i; };
> +
> +struct box __attribute__((noinline,noclone,noipa))
> +returns_struct (int i)
> +{
> +  struct box b;
> +  b.i = i * i;
> +  return b;
> +}
> +
> +int __attribute__((noinline,noclone))
> +test_1 (int i)
> +{
> +  [[gnu::musttail]] return returns_struct (i * 5).i; /* { dg-error "cannot tail-call: " } */
> +}
> diff --git a/gcc/testsuite/c-c++-common/musttail5.c b/gcc/testsuite/c-c++-common/musttail5.c
> new file mode 100644
> index 000000000000..71f4de40fc6d
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/musttail5.c
> @@ -0,0 +1,25 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=c23" { target c } } */
> +/* { dg-options "-std=gnu++11" { target c++ } } */
> +
> +[[musttail]] int j; /* { dg-warning "attribute" } */
> +__attribute__((musttail)) int k; /* { dg-warning "attribute" } */
> +
> +void foo(void)
> +{
> +  [[gnu::musttail]] j++; /* { dg-warning "attribute" } */
> +  [[gnu::musttail]] if (k > 0) /* { dg-warning "attribute" } */
> +    [[gnu::musttail]] k++; /* { dg-warning "attribute" } */
> +}
> +
> +int foo2(int p)
> +{
> +  [[gnu::musttail(1)]] return foo2(p + 1); /* { dg-error "attribute" } */
> +}
> +
> +int i;
> +
> +void foo3(void)
> +{
> +  [[musttail]] i++; /* { dg-warning "attribute" } */
> +}
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Updated musttail patches
  2024-02-02  9:09 Updated musttail patches Andi Kleen
                   ` (4 preceding siblings ...)
  2024-02-02  9:09 ` [PATCH v4 5/5] Add documentation for musttail attribute Andi Kleen
@ 2024-02-02 13:18 ` Joseph Myers
  2024-02-02 14:49   ` Andi Kleen
  5 siblings, 1 reply; 12+ messages in thread
From: Joseph Myers @ 2024-02-02 13:18 UTC (permalink / raw)
  To: Andi Kleen; +Cc: gcc-patches

On Fri, 2 Feb 2024, Andi Kleen wrote:

> This patchkit implements a [[musttail]] attribute for C/C++.
> 
> v4:
> Addressed all feedback except clang::musttail is still supported
> (I don't want to force an #ifdef to most users) and I'm also still

I'm fine with supporting [[clang::musttail]].  What shouldn't be supported 
is plain unnamespaced [[musttail]], at least for C, and I don't see any 
tests that that's not supported (there are tests of [[musttail]] on things 
that aren't returns, but that's mixing two different issues - 
[[gnu::musttail]] shouldn't be accepted on non-returns, while [[musttail]] 
shouldn't be accepted anywhere, including on returns, because it's not a 
standard attribute).

-- 
Joseph S. Myers
josmyers@redhat.com


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Updated musttail patches
  2024-02-02 13:18 ` Updated musttail patches Joseph Myers
@ 2024-02-02 14:49   ` Andi Kleen
  0 siblings, 0 replies; 12+ messages in thread
From: Andi Kleen @ 2024-02-02 14:49 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches

On Fri, Feb 02, 2024 at 01:18:06PM +0000, Joseph Myers wrote:
> On Fri, 2 Feb 2024, Andi Kleen wrote:
> 
> > This patchkit implements a [[musttail]] attribute for C/C++.
> > 
> > v4:
> > Addressed all feedback except clang::musttail is still supported
> > (I don't want to force an #ifdef to most users) and I'm also still
> 
> I'm fine with supporting [[clang::musttail]].  What shouldn't be supported 
> is plain unnamespaced [[musttail]], at least for C, and I don't see any 

It's unsupported since v4 ("" -> "gnu" for the lookups) 

> tests that that's not supported (there are tests of [[musttail]] on things 
> that aren't returns, but that's mixing two different issues - 
> [[gnu::musttail]] shouldn't be accepted on non-returns, while [[musttail]] 
> shouldn't be accepted anywhere, including on returns, because it's not a 
> standard attribute).

I added some extra tests for this  (passing)

diff --git a/gcc/testsuite/c-c++-common/musttail5.c b/gcc/testsuite/c-c++-common/musttail5.c
index 71f4de40fc6d..7938e7ff80e4 100644
--- a/gcc/testsuite/c-c++-common/musttail5.c
+++ b/gcc/testsuite/c-c++-common/musttail5.c
@@ -19,7 +19,10 @@ int foo2(int p)
 
 int i;
 
-void foo3(void)
+int foo3(void)
 {
   [[musttail]] i++; /* { dg-warning "attribute" } */
+  [[musttail]] if (i > 10) /* { dg-warning "attribute" } */
+    [[musttail]] return foo2(i); /* { dg-warning "attribute" } */
+  return 0;
 }



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 4/5] Add tests for C/C++ musttail attributes
  2024-02-02 11:01   ` Prathamesh Kulkarni
@ 2024-02-02 14:50     ` Andi Kleen
  0 siblings, 0 replies; 12+ messages in thread
From: Andi Kleen @ 2024-02-02 14:50 UTC (permalink / raw)
  To: Prathamesh Kulkarni; +Cc: gcc-patches

> Sorry, I wasn't clear about this in previous patch -- noipa will
> subsume other ipa attributes,
> so there's no need to have noinline, noclone along with noipa.
> int __attribute__((noipa)) callee(int i) should be sufficient for
> disabling IPA optimizations involving callee.

I thought you were worried about extra IPA optimizations. I prefer to
clearly say what I mean (besides it was just copied from existing
tests), so the verbose form is better. So if e.g. gcc ever re-adds the old
RTL inliner it wouldn't break. 

-Andi


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 5/5] Add documentation for musttail attribute
  2024-02-02  9:09 ` [PATCH v4 5/5] Add documentation for musttail attribute Andi Kleen
@ 2024-02-04  4:35   ` Sandra Loosemore
  2024-02-05 13:32     ` Andi Kleen
  0 siblings, 1 reply; 12+ messages in thread
From: Sandra Loosemore @ 2024-02-04  4:35 UTC (permalink / raw)
  To: Andi Kleen, gcc-patches

On 2/2/24 02:09, Andi Kleen wrote:
> gcc/ChangeLog:
> 
> 	* doc/extend.texi: Document [[musttail]]
> ---
>   gcc/doc/extend.texi | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 142e41ab8fbf..866f6c4a9fed 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -9875,6 +9875,22 @@ foo (int x, int y)
>   @code{y} is not actually incremented and the compiler can but does not
>   have to optimize it to just @code{return 42 + 42;}.
>   
> +@cindex @code{musttail} statement attribute
> +@item musttail
> +
> +The @code{gnu::musttail} or @code{clang::musttail} attribute
> +can be applied to a return statement that returns the value
> +of a call to indicate that the call must be a tail call
> +that does not allocate extra stack space.

It took me about 3 attempts to parse this.  :-S  I think this might be a 
little better:

...can be applied to a @code{return} statement with a return-value 
expression that is a function call.  It asserts that the call must be a 
tail call that does not allocate extra stack space.

> +
> +@smallexample
> +[[gnu::musttail]] return foo();
> +@end smallexample
> +
> +If the compiler cannot generate a tail call it will generate

s/will generate/generates/

I'm a big fan of writing in the present tense.  ;-)

> +an error. Tail calls generally require enabling optimization.
> +On some targets they may not be supported.
> +
>   @end table
>   
>   @node Attribute Syntax

In addition to these changes, at the beginning of this section we have

@node Statement Attributes
@section Statement Attributes
@cindex Statement Attributes

GCC allows attributes to be set on null statements.  @xref{Attribute 
Syntax},
for details of the exact syntax for using attributes. [...]

Well, we now have an attribute that goes on a non-null statement, so we 
have to fix this.  The documentation for the other statement attributes 
is already explicit that they go on null statements so those already 
would be OK if we just removed the "null" restriction here.  OTOH, the 
Attribute Syntax section, in discussing GCC's traditional attribute 
syntax, says:

@subsubheading Statement Attributes
In GNU C, an attribute specifier list may appear as part of a null
statement.  The attribute goes before the semicolon.

If "musttail" is only supported in the standard attribute syntax, its 
new entry in the Statement Attributes node must say that, and the blurb 
at the top of the node quoted above must say something to the effect 
that the traditional syntax only allows statement attributes on null 
statements and attributes on non-null statements are only permitted in 
the new standard attribute form.

-Sandra


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 5/5] Add documentation for musttail attribute
  2024-02-04  4:35   ` Sandra Loosemore
@ 2024-02-05 13:32     ` Andi Kleen
  0 siblings, 0 replies; 12+ messages in thread
From: Andi Kleen @ 2024-02-05 13:32 UTC (permalink / raw)
  To: Sandra Loosemore; +Cc: gcc-patches

On Sat, Feb 03, 2024 at 09:35:43PM -0700, Sandra Loosemore wrote:
> On 2/2/24 02:09, Andi Kleen wrote:
> > gcc/ChangeLog:
> > 
> > 	* doc/extend.texi: Document [[musttail]]
> > ---
> >   gcc/doc/extend.texi | 16 ++++++++++++++++
> >   1 file changed, 16 insertions(+)
> > 
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index 142e41ab8fbf..866f6c4a9fed 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -9875,6 +9875,22 @@ foo (int x, int y)
> >   @code{y} is not actually incremented and the compiler can but does not
> >   have to optimize it to just @code{return 42 + 42;}.
> > +@cindex @code{musttail} statement attribute
> > +@item musttail
> > +
> > +The @code{gnu::musttail} or @code{clang::musttail} attribute
> > +can be applied to a return statement that returns the value
> > +of a call to indicate that the call must be a tail call
> > +that does not allocate extra stack space.
> 
> It took me about 3 attempts to parse this.  :-S  I think this might be a
> little better:
> 
> ...can be applied to a @code{return} statement with a return-value
> expression that is a function call.  It asserts that the call must be a tail
> call that does not allocate extra stack space.
> 
> > +
> > +@smallexample
> > +[[gnu::musttail]] return foo();
> > +@end smallexample
> > +
> > +If the compiler cannot generate a tail call it will generate
> 
> s/will generate/generates/
> 
> I'm a big fan of writing in the present tense.  ;-)
> 
> > +an error. Tail calls generally require enabling optimization.
> > +On some targets they may not be supported.
> > +
> >   @end table
> >   @node Attribute Syntax
> 
> In addition to these changes, at the beginning of this section we have
> 
> @node Statement Attributes
> @section Statement Attributes
> @cindex Statement Attributes
> 
> GCC allows attributes to be set on null statements.  @xref{Attribute
> Syntax},
> for details of the exact syntax for using attributes. [...]
> 
> Well, we now have an attribute that goes on a non-null statement, so we have
> to fix this.  The documentation for the other statement attributes is

FWIW we always had, they just were ignored (with a warning)


Thanks Sandra. I applied the changes. Diff appeneded for reference.


diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 866f6c4a9fed..fe1ee245ed69 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9818,7 +9818,7 @@ same manner as the @code{deprecated} attribute.
 @section Statement Attributes
 @cindex Statement Attributes
 
-GCC allows attributes to be set on null statements.  @xref{Attribute Syntax},
+GCC allows attributes to be set on statements.  @xref{Attribute Syntax},
 for details of the exact syntax for using attributes.  Other attributes are
 available for functions (@pxref{Function Attributes}), variables
 (@pxref{Variable Attributes}), labels (@pxref{Label Attributes}), enumerators
@@ -9879,15 +9879,15 @@ have to optimize it to just @code{return 42 + 42;}.
 @item musttail
 
 The @code{gnu::musttail} or @code{clang::musttail} attribute
-can be applied to a return statement that returns the value
-of a call to indicate that the call must be a tail call
-that does not allocate extra stack space.
+can be applied to a @code{return} statement with a return-value expression
+that is a function call.  It asserts that the call must be a tail call that
+does not allocate extra stack space.
 
 @smallexample
 [[gnu::musttail]] return foo();
 @end smallexample
 
-If the compiler cannot generate a tail call it will generate
+If the compiler cannot generate a tail call it generates
 an error. Tail calls generally require enabling optimization.
 On some targets they may not be supported.
 
@@ -10014,7 +10014,9 @@ the constant expression, if present.
 
 @subsubheading Statement Attributes
 In GNU C, an attribute specifier list may appear as part of a null
-statement.  The attribute goes before the semicolon.
+statement. The attribute goes before the semicolon.
+Some attributes in new style syntax are also supported
+on non-null statements.
 
 @subsubheading Type Attributes
 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-02-05 13:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-02  9:09 Updated musttail patches Andi Kleen
2024-02-02  9:09 ` [PATCH v4 1/5] Improve must tail in RTL backend Andi Kleen
2024-02-02  9:09 ` [PATCH v4 2/5] C++: Support clang compatible [[musttail]] (PR83324) Andi Kleen
2024-02-02  9:09 ` [PATCH v4 3/5] C: Implement musttail attribute for returns Andi Kleen
2024-02-02  9:09 ` [PATCH v4 4/5] Add tests for C/C++ musttail attributes Andi Kleen
2024-02-02 11:01   ` Prathamesh Kulkarni
2024-02-02 14:50     ` Andi Kleen
2024-02-02  9:09 ` [PATCH v4 5/5] Add documentation for musttail attribute Andi Kleen
2024-02-04  4:35   ` Sandra Loosemore
2024-02-05 13:32     ` Andi Kleen
2024-02-02 13:18 ` Updated musttail patches Joseph Myers
2024-02-02 14:49   ` Andi Kleen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).