On 5/5/24 14:14, Andi Kleen wrote:
This patch implements a clang compatible [[musttail]] attribute for
returns.

Thanks.

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

@@ -12734,9 +12734,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);

It seems to me that if we were to pass &std_attrs to cp_parser_jump_statement, we could handle this entirely in that function rather than adding a flag to finish_return_stmt and check_return_stmt.

@@ -30189,7 +30207,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))

I'd prefer to add an attribute to the table, rather than special-case it here; apart from consistency, it seems likely that someone will later want to apply it to a function.

You need a template testcase; I expect it doesn't work in templates with the current patch. It's probably enough to copy it in tsubst_expr where we currently propagate CALL_EXPR_OPERATOR_SYNTAX.

You also need a testcase where the function returns a class; in that case the call will often appear as AGGR_INIT_EXPR rather than CALL_EXPR, so you'll need to handle that as well. And see the places that copy flags like CALL_EXPR_OPERATOR_SYNTAX between CALL_EXPR and AGGR_INIT_EXPR.

Jason

Reply via email to