[v7,4/9] C++: Support clang compatible [[musttail]] (PR83324)

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:

	* parser.cc (cp_parser_statement): Handle musttail.
	(cp_parser_jump_statement): Dito.
	(cp_parser_std_attribute): Dito.
	* pt.cc (tsubst_expr): Copy CALL_EXPR_MUST_TAIL_CALL.
---
 gcc/cp/parser.cc | 42 +++++++++++++++++++++++++++++++++++++-----
 gcc/cp/pt.cc     |  4 +++-
 2 files changed, 40 insertions(+), 6 deletions(-)

On 6/2/24 13:16, Andi Kleen wrote:
> 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:
> 
> 	* parser.cc (cp_parser_statement): Handle musttail.
> 	(cp_parser_jump_statement): Dito.
> 	(cp_parser_std_attribute): Dito.
> 	* pt.cc (tsubst_expr): Copy CALL_EXPR_MUST_TAIL_CALL.
> ---
>   gcc/cp/parser.cc | 42 +++++++++++++++++++++++++++++++++++++-----
>   gcc/cp/pt.cc     |  4 +++-
>   2 files changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 779625144db4..c2cb304bac7d 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -2467,7 +2467,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 *, tree &);
>   static void cp_parser_declaration_statement
>     (cp_parser *);
>   
> @@ -12747,13 +12747,17 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>   						     NULL_TREE, false);
>   	  break;
>   
> +	case RID_RETURN:
> +	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
> +	  statement = cp_parser_jump_statement (parser, std_attrs);
> +	  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);
> -	  statement = cp_parser_jump_statement (parser);
> +	  statement = cp_parser_jump_statement (parser, std_attrs);
>   	  break;
>   
>   	  /* Objective-C++ exception-handling constructs.  */
> @@ -14813,10 +14817,11 @@ cp_parser_init_statement (cp_parser *parser, tree *decl)
>      jump-statement:
>        goto * expression ;
>   
> +   STD_ATTRS are the statement attributes. They can be modified.
>      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, tree &std_attrs)
>   {
>     tree statement = error_mark_node;
>     cp_token *token;
> @@ -14893,6 +14898,33 @@ cp_parser_jump_statement (cp_parser* parser)
>   	  /* If the next token is a `;', then there is no
>   	     expression.  */
>   	  expr = NULL_TREE;
> +
> +	if (keyword == RID_RETURN && expr)
> +	  {
> +	    bool musttail_p = false;
> +	    if (lookup_attribute ("gnu", "musttail", std_attrs))
> +	      {
> +		musttail_p = true;
> +		std_attrs = remove_attribute ("gnu", "musttail", std_attrs);
> +	      }
> +	    // support this for compatibility

This should follow the usual comment conventions, i.e.

/* Support this for compatibility.  */

> +	    if (lookup_attribute ("clang", "musttail", std_attrs))
> +	      {
> +		musttail_p = true;
> +		std_attrs = remove_attribute ("clang", "musttail", std_attrs);
> +	      }
> +	    if (musttail_p)
> +	      {
> +		tree t = expr;
> +		if (t && TREE_CODE (t) == TARGET_EXPR)
> +		  t = TARGET_EXPR_INITIAL (t);
> +		if (t && TREE_CODE (t) != CALL_EXPR)
> +		  error_at (token->location, "cannot tail-call: return value must be a call");
> +		else
> +		  CALL_EXPR_MUST_TAIL_CALL (t) = 1;
> +	      }
> +	  }
> +
>   	/* Build the return-statement, check co-return first, since type
>   	   deduction is not valid there.  */
>   	if (keyword == RID_CO_RETURN)
> @@ -30316,7 +30348,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))

This shouldn't be necessary with the attribute in the c-attribs table, 
right?  This patch is OK without this hunk and with the comment tweak above.

Jason

On Mon, Jun 03, 2024 at 10:42:20AM -0400, Jason Merrill wrote:
> > @@ -30316,7 +30348,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))
> 
> This shouldn't be necessary with the attribute in the c-attribs table,
> right?  This patch is OK without this hunk and with the comment tweak above.

Yes I will remove it. Also the hunk above can be simplified, we don't
need the extra case anymore.

But unfortunately there's another problem (sorry I missed that earlier
but the Linaro bot pointed it out again):

This hunk:

@@ -21085,12 +21085,14 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    bool op = CALL_EXPR_OPERATOR_SYNTAX (t);
 	    bool ord = CALL_EXPR_ORDERED_ARGS (t);
 	    bool rev = CALL_EXPR_REVERSE_ARGS (t);
-	    if (op || ord || rev)
+	    bool mtc = CALL_EXPR_MUST_TAIL_CALL (t);
+	    if (op || ord || rev || mtc)
 	      if (tree call = extract_call_expr (ret))
 		{
 		  CALL_EXPR_OPERATOR_SYNTAX (call) = op;
 		  CALL_EXPR_ORDERED_ARGS (call) = ord;
 		  CALL_EXPR_REVERSE_ARGS (call) = rev;
+		  CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
 		}
 	    if (warning_suppressed_p (t, OPT_Wpessimizing_move))
 	      /* This also suppresses -Wredundant-move.  */


causes 

/home/ak/gcc/gcc/gcc/testsuite/g++.dg/ipa/devirt-52.C:49:8: internal
compiler error: tree check: expected call_expr, have aggr_init_expr in
tsubst_expr, at cp/pt.cc:21095
0x910b66 tree_check_failed(tree_node const*, char const*, int, char
const*, ...)
        ../../gcc/gcc/tree.cc:8995


I suspect that's because CALL_EXPR_MUST_TAIL_CALL uses
tree_core->static_flag, but that means something else during
template expansion?

Any suggestions how to fix that?

-Andi

On Mon, Jun 03, 2024 at 08:33:52AM -0700, Andi Kleen wrote:
> On Mon, Jun 03, 2024 at 10:42:20AM -0400, Jason Merrill wrote:
> > > @@ -30316,7 +30348,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))
> > 
> > This shouldn't be necessary with the attribute in the c-attribs table,
> > right?  This patch is OK without this hunk and with the comment tweak above.
> 
> Yes I will remove it. Also the hunk above can be simplified, we don't
> need the extra case anymore.
> 
> But unfortunately there's another problem (sorry I missed that earlier
> but the Linaro bot pointed it out again):
> 
> This hunk:
> 
> @@ -21085,12 +21085,14 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>  	    bool op = CALL_EXPR_OPERATOR_SYNTAX (t);
>  	    bool ord = CALL_EXPR_ORDERED_ARGS (t);
>  	    bool rev = CALL_EXPR_REVERSE_ARGS (t);
> -	    if (op || ord || rev)
> +	    bool mtc = CALL_EXPR_MUST_TAIL_CALL (t);
> +	    if (op || ord || rev || mtc)
>  	      if (tree call = extract_call_expr (ret))
>  		{
>  		  CALL_EXPR_OPERATOR_SYNTAX (call) = op;
>  		  CALL_EXPR_ORDERED_ARGS (call) = ord;
>  		  CALL_EXPR_REVERSE_ARGS (call) = rev;
> +		  CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
>  		}

The difference is that CALL_EXPR_MUST_TAIL_CALL is defined as:
#define CALL_EXPR_MUST_TAIL_CALL(NODE) \
  (CALL_EXPR_CHECK (NODE)->base.static_flag)
while the others like:
#define CALL_EXPR_ORDERED_ARGS(NODE) \
  TREE_LANG_FLAG_3 (CALL_OR_AGGR_INIT_CHECK (NODE))
where
#define CALL_OR_AGGR_INIT_CHECK(NODE) \
  TREE_CHECK2 ((NODE), CALL_EXPR, AGGR_INIT_EXPR)
while
#define CALL_EXPR_CHECK(t)	TREE_CHECK (t, CALL_EXPR)
(this one is defined in generated tree-check.h).
So, while the CALL_EXPR_REVERSE_ARGS etc. can be used on either
CALL_EXPR or AGGR_INIT_EXPR (the latter is a C++ specific tree code),
CALL_EXPR_MUST_TAIL_CALL is allowed only on CALL_EXPR.
AGGR_INIT_EXPR is used for C++ constructor calls, so I think
they really don't need such a flag, so you could do:
	    bool mtc = (TREE_CODE (t) == CALL_EXPR
			? CALL_EXPR_MUST_TAIL_CALL (t) : false);
	    if (op || ord || rev || mtc)
...
	      if (mtc)
		CALL_EXPR_MUST_TAIL_CALL (call) = 1;
or something similar.
Or you'd need to define a variant of the CALL_EXPR_MUST_TAIL_CALL
macro for the C++ FE (as CALL_OR_AGGR_INIT_CHECK is C++ FE too)
and use that in the FE and somehow assert it means the same thing
as the middle-end flag except that it can be also used on AGGR_INIT_EXPR.

	Jakub

On 6/3/24 11:44, Jakub Jelinek wrote:
> On Mon, Jun 03, 2024 at 08:33:52AM -0700, Andi Kleen wrote:
>> On Mon, Jun 03, 2024 at 10:42:20AM -0400, Jason Merrill wrote:
>>>> @@ -30316,7 +30348,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))
>>>
>>> This shouldn't be necessary with the attribute in the c-attribs table,
>>> right?  This patch is OK without this hunk and with the comment tweak above.
>>
>> Yes I will remove it. Also the hunk above can be simplified, we don't
>> need the extra case anymore.
>>
>> But unfortunately there's another problem (sorry I missed that earlier
>> but the Linaro bot pointed it out again):
>>
>> This hunk:
>>
>> @@ -21085,12 +21085,14 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>>   	    bool op = CALL_EXPR_OPERATOR_SYNTAX (t);
>>   	    bool ord = CALL_EXPR_ORDERED_ARGS (t);
>>   	    bool rev = CALL_EXPR_REVERSE_ARGS (t);
>> -	    if (op || ord || rev)
>> +	    bool mtc = CALL_EXPR_MUST_TAIL_CALL (t);
>> +	    if (op || ord || rev || mtc)
>>   	      if (tree call = extract_call_expr (ret))
>>   		{
>>   		  CALL_EXPR_OPERATOR_SYNTAX (call) = op;
>>   		  CALL_EXPR_ORDERED_ARGS (call) = ord;
>>   		  CALL_EXPR_REVERSE_ARGS (call) = rev;
>> +		  CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
>>   		}
> 
> The difference is that CALL_EXPR_MUST_TAIL_CALL is defined as:
> #define CALL_EXPR_MUST_TAIL_CALL(NODE) \
>    (CALL_EXPR_CHECK (NODE)->base.static_flag)
> while the others like:
> #define CALL_EXPR_ORDERED_ARGS(NODE) \
>    TREE_LANG_FLAG_3 (CALL_OR_AGGR_INIT_CHECK (NODE))
> where
> #define CALL_OR_AGGR_INIT_CHECK(NODE) \
>    TREE_CHECK2 ((NODE), CALL_EXPR, AGGR_INIT_EXPR)
> while
> #define CALL_EXPR_CHECK(t)	TREE_CHECK (t, CALL_EXPR)
> (this one is defined in generated tree-check.h).
> So, while the CALL_EXPR_REVERSE_ARGS etc. can be used on either
> CALL_EXPR or AGGR_INIT_EXPR (the latter is a C++ specific tree code),
> CALL_EXPR_MUST_TAIL_CALL is allowed only on CALL_EXPR.
> AGGR_INIT_EXPR is used for C++ constructor calls, so I think
> they really don't need such a flag

AGGR_INIT_EXPR is also used for functions returning a class, so I think 
it is needed.

Jason

On Mon, Jun 03, 2024 at 12:29:28PM -0400, Jason Merrill wrote:
> On 6/3/24 11:44, Jakub Jelinek wrote:
> > On Mon, Jun 03, 2024 at 08:33:52AM -0700, Andi Kleen wrote:
> > > On Mon, Jun 03, 2024 at 10:42:20AM -0400, Jason Merrill wrote:
> > > > > @@ -30316,7 +30348,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))
> > > > 
> > > > This shouldn't be necessary with the attribute in the c-attribs table,
> > > > right?  This patch is OK without this hunk and with the comment tweak above.
> > > 
> > > Yes I will remove it. Also the hunk above can be simplified, we don't
> > > need the extra case anymore.
> > > 
> > > But unfortunately there's another problem (sorry I missed that earlier
> > > but the Linaro bot pointed it out again):
> > > 
> > > This hunk:
> > > 
> > > @@ -21085,12 +21085,14 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
> > >   	    bool op = CALL_EXPR_OPERATOR_SYNTAX (t);
> > >   	    bool ord = CALL_EXPR_ORDERED_ARGS (t);
> > >   	    bool rev = CALL_EXPR_REVERSE_ARGS (t);
> > > -	    if (op || ord || rev)
> > > +	    bool mtc = CALL_EXPR_MUST_TAIL_CALL (t);
> > > +	    if (op || ord || rev || mtc)
> > >   	      if (tree call = extract_call_expr (ret))
> > >   		{
> > >   		  CALL_EXPR_OPERATOR_SYNTAX (call) = op;
> > >   		  CALL_EXPR_ORDERED_ARGS (call) = ord;
> > >   		  CALL_EXPR_REVERSE_ARGS (call) = rev;
> > > +		  CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
> > >   		}
> > 
> > The difference is that CALL_EXPR_MUST_TAIL_CALL is defined as:
> > #define CALL_EXPR_MUST_TAIL_CALL(NODE) \
> >    (CALL_EXPR_CHECK (NODE)->base.static_flag)
> > while the others like:
> > #define CALL_EXPR_ORDERED_ARGS(NODE) \
> >    TREE_LANG_FLAG_3 (CALL_OR_AGGR_INIT_CHECK (NODE))
> > where
> > #define CALL_OR_AGGR_INIT_CHECK(NODE) \
> >    TREE_CHECK2 ((NODE), CALL_EXPR, AGGR_INIT_EXPR)
> > while
> > #define CALL_EXPR_CHECK(t)	TREE_CHECK (t, CALL_EXPR)
> > (this one is defined in generated tree-check.h).
> > So, while the CALL_EXPR_REVERSE_ARGS etc. can be used on either
> > CALL_EXPR or AGGR_INIT_EXPR (the latter is a C++ specific tree code),
> > CALL_EXPR_MUST_TAIL_CALL is allowed only on CALL_EXPR.
> > AGGR_INIT_EXPR is used for C++ constructor calls, so I think
> > they really don't need such a flag
> 
> AGGR_INIT_EXPR is also used for functions returning a class, so I think it
> is needed.

I used this variant which passes tests. It assumes that there are no
wrapped calls with this flag, but I assume that's ok.

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index f6af8e1a81e4..c50fea4282e4 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -21085,14 +21085,17 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    bool op = CALL_EXPR_OPERATOR_SYNTAX (t);
 	    bool ord = CALL_EXPR_ORDERED_ARGS (t);
 	    bool rev = CALL_EXPR_REVERSE_ARGS (t);
-	    bool mtc = CALL_EXPR_MUST_TAIL_CALL (t);
+	    bool mtc = false;
+	    if (TREE_CODE (t) == CALL_EXPR)
+	      mtc = CALL_EXPR_MUST_TAIL_CALL (t);
 	    if (op || ord || rev || mtc)
 	      if (tree call = extract_call_expr (ret))
 		{
 		  CALL_EXPR_OPERATOR_SYNTAX (call) = op;
 		  CALL_EXPR_ORDERED_ARGS (call) = ord;
 		  CALL_EXPR_REVERSE_ARGS (call) = rev;
-		  CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
+		  if (TREE_CODE (call) == CALL_EXPR)
+		    CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
 		}
 	    if (warning_suppressed_p (t, OPT_Wpessimizing_move))
 	      /* This also suppresses -Wredundant-move.  */

On 6/3/24 15:35, Andi Kleen wrote:
> On Mon, Jun 03, 2024 at 12:29:28PM -0400, Jason Merrill wrote:
>> On 6/3/24 11:44, Jakub Jelinek wrote:
>>> On Mon, Jun 03, 2024 at 08:33:52AM -0700, Andi Kleen wrote:
>>>> On Mon, Jun 03, 2024 at 10:42:20AM -0400, Jason Merrill wrote:
>>>>>> @@ -30316,7 +30348,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))
>>>>>
>>>>> This shouldn't be necessary with the attribute in the c-attribs table,
>>>>> right?  This patch is OK without this hunk and with the comment tweak above.
>>>>
>>>> Yes I will remove it. Also the hunk above can be simplified, we don't
>>>> need the extra case anymore.
>>>>
>>>> But unfortunately there's another problem (sorry I missed that earlier
>>>> but the Linaro bot pointed it out again):
>>>>
>>>> This hunk:
>>>>
>>>> @@ -21085,12 +21085,14 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>>>>    	    bool op = CALL_EXPR_OPERATOR_SYNTAX (t);
>>>>    	    bool ord = CALL_EXPR_ORDERED_ARGS (t);
>>>>    	    bool rev = CALL_EXPR_REVERSE_ARGS (t);
>>>> -	    if (op || ord || rev)
>>>> +	    bool mtc = CALL_EXPR_MUST_TAIL_CALL (t);
>>>> +	    if (op || ord || rev || mtc)
>>>>    	      if (tree call = extract_call_expr (ret))
>>>>    		{
>>>>    		  CALL_EXPR_OPERATOR_SYNTAX (call) = op;
>>>>    		  CALL_EXPR_ORDERED_ARGS (call) = ord;
>>>>    		  CALL_EXPR_REVERSE_ARGS (call) = rev;
>>>> +		  CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
>>>>    		}
>>>
>>> The difference is that CALL_EXPR_MUST_TAIL_CALL is defined as:
>>> #define CALL_EXPR_MUST_TAIL_CALL(NODE) \
>>>     (CALL_EXPR_CHECK (NODE)->base.static_flag)
>>> while the others like:
>>> #define CALL_EXPR_ORDERED_ARGS(NODE) \
>>>     TREE_LANG_FLAG_3 (CALL_OR_AGGR_INIT_CHECK (NODE))
>>> where
>>> #define CALL_OR_AGGR_INIT_CHECK(NODE) \
>>>     TREE_CHECK2 ((NODE), CALL_EXPR, AGGR_INIT_EXPR)
>>> while
>>> #define CALL_EXPR_CHECK(t)	TREE_CHECK (t, CALL_EXPR)
>>> (this one is defined in generated tree-check.h).
>>> So, while the CALL_EXPR_REVERSE_ARGS etc. can be used on either
>>> CALL_EXPR or AGGR_INIT_EXPR (the latter is a C++ specific tree code),
>>> CALL_EXPR_MUST_TAIL_CALL is allowed only on CALL_EXPR.
>>> AGGR_INIT_EXPR is used for C++ constructor calls, so I think
>>> they really don't need such a flag
>>
>> AGGR_INIT_EXPR is also used for functions returning a class, so I think it
>> is needed.
> 
> I used this variant which passes tests. It assumes that there are no
> wrapped calls with this flag, but I assume that's ok.

musttail10.C should also check the case where the function called with 
[[musttail]] returns a non-trivially-copyable class (e.g. that has a 
destructor).

> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index f6af8e1a81e4..c50fea4282e4 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -21085,14 +21085,17 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   	    bool op = CALL_EXPR_OPERATOR_SYNTAX (t);
>   	    bool ord = CALL_EXPR_ORDERED_ARGS (t);
>   	    bool rev = CALL_EXPR_REVERSE_ARGS (t);
> -	    bool mtc = CALL_EXPR_MUST_TAIL_CALL (t);
> +	    bool mtc = false;
> +	    if (TREE_CODE (t) == CALL_EXPR)
> +	      mtc = CALL_EXPR_MUST_TAIL_CALL (t);
>   	    if (op || ord || rev || mtc)
>   	      if (tree call = extract_call_expr (ret))
>   		{
>   		  CALL_EXPR_OPERATOR_SYNTAX (call) = op;
>   		  CALL_EXPR_ORDERED_ARGS (call) = ord;
>   		  CALL_EXPR_REVERSE_ARGS (call) = rev;
> -		  CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
> +		  if (TREE_CODE (call) == CALL_EXPR)
> +		    CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
>   		}
>   	    if (warning_suppressed_p (t, OPT_Wpessimizing_move))
>   	      /* This also suppresses -Wredundant-move.  */
>

> > I used this variant which passes tests. It assumes that there are no
> > wrapped calls with this flag, but I assume that's ok.
> 
> musttail10.C should also check the case where the function called with
> [[musttail]] returns a non-trivially-copyable class (e.g. that has a
> destructor).

It fails as expected, however the resulting tree code confuses
tree-tailcall, so the end result is a not very helpful
"cannot tail-call: other reasons" error when only expand
catches it.

I may fix that later, but I don't think it's critical for the
initial feature. There are some other cases where this happens too.

I also improved the test case by running it in "torture" to cover
more cases

-Andi