[v9,08/10] Add tests for C/C++ musttail attributes

  Some 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/musttail7.c: New test.
	* c-c++-common/musttail8.c: New test.
	* g++.dg/musttail6.C: New test.
	* g++.dg/musttail9.C: New test.
	* g++.dg/musttail10.C: New test.
---
 gcc/testsuite/c-c++-common/musttail1.c | 14 ++++++
 gcc/testsuite/c-c++-common/musttail2.c | 33 ++++++++++++++
 gcc/testsuite/c-c++-common/musttail3.c | 29 ++++++++++++
 gcc/testsuite/c-c++-common/musttail4.c | 17 +++++++
 gcc/testsuite/c-c++-common/musttail5.c | 28 ++++++++++++
 gcc/testsuite/c-c++-common/musttail7.c | 14 ++++++
 gcc/testsuite/c-c++-common/musttail8.c | 17 +++++++
 gcc/testsuite/g++.dg/musttail10.C      | 34 ++++++++++++++
 gcc/testsuite/g++.dg/musttail6.C       | 61 ++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/musttail9.C       | 10 +++++
 10 files changed, 257 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
 create mode 100644 gcc/testsuite/c-c++-common/musttail7.c
 create mode 100644 gcc/testsuite/c-c++-common/musttail8.c
 create mode 100644 gcc/testsuite/g++.dg/musttail10.C
 create mode 100644 gcc/testsuite/g++.dg/musttail6.C
 create mode 100644 gcc/testsuite/g++.dg/musttail9.C
  

On 7/8/24 12:56 PM, Andi Kleen wrote:
> diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
> new file mode 100644
> index 000000000000..9b7043b8a306
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/musttail10.C
> @@ -0,0 +1,34 @@
> +/* { dg-do compile { target { tail_call } } } */
> +/* { dg-options "-std=gnu++11" } */
> +/* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
> +
> +int f();
> +
> +double h() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
> +
> +template <class T>
> +__attribute__((noinline, noclone, noipa))
> +T g1() { [[gnu::musttail]] return f(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */
> +
> +template <class T>
> +__attribute__((noinline, noclone, noipa))
> +T g2() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
> +
> +template <class T>
> +__attribute__((noinline, noclone, noipa))
> +T g3() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
> +
> +class C
> +{
> +  double x;
> +public:
> +  C(double x) : x(x) {}
> +  ~C() { asm("":::"memory"); }
> +};
> +
> +int main()
> +{
> +  g1<int>();
> +  g2<double>();
> +  g3<C>();
> +}

I had asked for this test to check the case where the function called 
with [[musttail]] returns a non-trivially-copyable class; the test now 
includes such a class, but all the [[musttail]] calls are still to a 
function that returns int.

Jason
  

On Mon, Jul 15, 2024 at 06:57:57PM -0400, Jason Merrill wrote:
> On 7/8/24 12:56 PM, Andi Kleen wrote:
> > diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
> > new file mode 100644
> > index 000000000000..9b7043b8a306
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/musttail10.C
> > @@ -0,0 +1,34 @@
> > +/* { dg-do compile { target { tail_call } } } */
> > +/* { dg-options "-std=gnu++11" } */
> > +/* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
> > +
> > +int f();
> > +
> > +double h() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
> > +
> > +template <class T>
> > +__attribute__((noinline, noclone, noipa))
> > +T g1() { [[gnu::musttail]] return f(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */
> > +
> > +template <class T>
> > +__attribute__((noinline, noclone, noipa))
> > +T g2() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
> > +
> > +template <class T>
> > +__attribute__((noinline, noclone, noipa))
> > +T g3() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
> > +
> > +class C
> > +{
> > +  double x;
> > +public:
> > +  C(double x) : x(x) {}
> > +  ~C() { asm("":::"memory"); }
> > +};
> > +
> > +int main()
> > +{
> > +  g1<int>();
> > +  g2<double>();
> > +  g3<C>();
> > +}
> 
> I had asked for this test to check the case where the function called with
> [[musttail]] returns a non-trivially-copyable class; the test now includes
> such a class, but all the [[musttail]] calls are still to a function that
> returns int.

Thanks Jason.

I fixed the test case, but now the musttail gets lost, no error for g2/g3.

That means the flag is still lost somewhere. Does something outside tsubst need changes too?

Right now tsubst has only this:

@@ -21113,12 +21113,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);
-           if (op || ord || rev)
+           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;
+                 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.  */


Fixed test case:


template <class T> T f();

double h() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-call" } */

template <class T>
__attribute__((noinline, noclone, noipa))
T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */

template <class T>
__attribute__((noinline, noclone, noipa))
T g2() { [[gnu::musttail]] return f<T>(); } /* { dg-error "cannot tail-call" } */

template <class T>
__attribute__((noinline, noclone, noipa))
T g3() { [[gnu::musttail]] return f<T>(); } /* { dg-error "cannot tail-call" } */

class C
{
  double x;
public:
  C(double x) : x(x) {}
  ~C() { asm("":::"memory"); }
};

int main()
{
  g1<int>();
  g2<double>();
  g3<C>();
}
  

On 7/15/24 11:24 PM, Andi Kleen wrote:
> On Mon, Jul 15, 2024 at 06:57:57PM -0400, Jason Merrill wrote:
>> On 7/8/24 12:56 PM, Andi Kleen wrote:
>>> diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
>>> new file mode 100644
>>> index 000000000000..9b7043b8a306
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/musttail10.C
>>> @@ -0,0 +1,34 @@
>>> +/* { dg-do compile { target { tail_call } } } */
>>> +/* { dg-options "-std=gnu++11" } */
>>> +/* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
>>> +
>>> +int f();
>>> +
>>> +double h() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
>>> +
>>> +template <class T>
>>> +__attribute__((noinline, noclone, noipa))
>>> +T g1() { [[gnu::musttail]] return f(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */
>>> +
>>> +template <class T>
>>> +__attribute__((noinline, noclone, noipa))
>>> +T g2() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
>>> +
>>> +template <class T>
>>> +__attribute__((noinline, noclone, noipa))
>>> +T g3() { [[gnu::musttail]] return f(); } /* { dg-error "cannot tail-call" } */
>>> +
>>> +class C
>>> +{
>>> +  double x;
>>> +public:
>>> +  C(double x) : x(x) {}
>>> +  ~C() { asm("":::"memory"); }
>>> +};
>>> +
>>> +int main()
>>> +{
>>> +  g1<int>();
>>> +  g2<double>();
>>> +  g3<C>();
>>> +}
>>
>> I had asked for this test to check the case where the function called with
>> [[musttail]] returns a non-trivially-copyable class; the test now includes
>> such a class, but all the [[musttail]] calls are still to a function that
>> returns int.
> 
> Thanks Jason.
> 
> I fixed the test case, but now the musttail gets lost, no error for g2/g3.
> 
> That means the flag is still lost somewhere. Does something outside tsubst need changes too?
> 
> Right now tsubst has only this:
> 
> @@ -21113,12 +21113,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);
> -           if (op || ord || rev)
> +           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;
> +                 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.  */
> 
> 
> Fixed test case:
> 
> 
> template <class T> T f();
> 
> double h() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-call" } */
> 
> template <class T>
> __attribute__((noinline, noclone, noipa))
> T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */
> 
> template <class T>
> __attribute__((noinline, noclone, noipa))
> T g2() { [[gnu::musttail]] return f<T>(); } /* { dg-error "cannot tail-call" } */
> 
> template <class T>
> __attribute__((noinline, noclone, noipa))
> T g3() { [[gnu::musttail]] return f<T>(); } /* { dg-error "cannot tail-call" } */

In the adjusted test it looks like the types of f and g match, so I 
wouldn't expect an error.

> class C
> {
>    double x;
> public:
>    C(double x) : x(x) {}
>    ~C() { asm("":::"memory"); }
> };
> 
> int main()
> {
>    g1<int>();
>    g2<double>();
>    g3<C>();
> }
>
  

> In the adjusted test it looks like the types of f and g match, so I wouldn't
> expect an error.

Good point! Missing the forest for the trees.

Anyways are the C++ patches ok with this change?

Thanks,
-Andi
  

On 7/16/24 11:15 AM, Andi Kleen wrote:
>> In the adjusted test it looks like the types of f and g match, so I wouldn't
>> expect an error.
> 
> Good point! Missing the forest for the trees.
> 
> Anyways are the C++ patches ok with this change?

I'm still looking for a test which does error because the types are 
different.

Jason
  

On Tue, Jul 16, 2024 at 11:17:14AM -0400, Jason Merrill wrote:
> On 7/16/24 11:15 AM, Andi Kleen wrote:
> > > In the adjusted test it looks like the types of f and g match, so I wouldn't
> > > expect an error.
> > 
> > Good point! Missing the forest for the trees.
> > 
> > Anyways are the C++ patches ok with this change?
> 
> I'm still looking for a test which does error because the types are
> different.

Like this?

-Andi


diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
index 6a8507784a14..e454a6238a06 100644
--- a/gcc/testsuite/g++.dg/musttail10.C
+++ b/gcc/testsuite/g++.dg/musttail10.C
@@ -4,7 +4,7 @@
 
 template <class T> T f();
 
-double h() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-call" } */
+double g() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-call" } */
 
 template <class T>
 __attribute__((noinline, noclone, noipa))
@@ -18,6 +18,10 @@ template <class T>
 __attribute__((noinline, noclone, noipa))
 T g3() { [[gnu::musttail]] return f<T>(); }
 
+template <class T>
+__attribute__((noinline, noclone, noipa))
+T g4() { [[gnu::musttail]] return f<double>(); } /* { dg-error "cannot tail-call" } */
+
 class C
 {
   double x;
@@ -31,4 +35,5 @@ int main()
   g1<int>();
   g2<double>();
   g3<C>();
+  g4<int>();
 }
  

On 7/16/24 12:18 PM, Andi Kleen wrote:
> On Tue, Jul 16, 2024 at 11:17:14AM -0400, Jason Merrill wrote:
>> On 7/16/24 11:15 AM, Andi Kleen wrote:
>>>> In the adjusted test it looks like the types of f and g match, so I wouldn't
>>>> expect an error.
>>>
>>> Good point! Missing the forest for the trees.
>>>
>>> Anyways are the C++ patches ok with this change?
>>
>> I'm still looking for a test which does error because the types are
>> different.
> 
> Like this?

Where the called function returns C and the callee function does not.

> -Andi
> 
> 
> diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
> index 6a8507784a14..e454a6238a06 100644
> --- a/gcc/testsuite/g++.dg/musttail10.C
> +++ b/gcc/testsuite/g++.dg/musttail10.C
> @@ -4,7 +4,7 @@
>   
>   template <class T> T f();
>   
> -double h() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-call" } */
> +double g() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-call" } */
>   
>   template <class T>
>   __attribute__((noinline, noclone, noipa))
> @@ -18,6 +18,10 @@ template <class T>
>   __attribute__((noinline, noclone, noipa))
>   T g3() { [[gnu::musttail]] return f<T>(); }
>   
> +template <class T>
> +__attribute__((noinline, noclone, noipa))
> +T g4() { [[gnu::musttail]] return f<double>(); } /* { dg-error "cannot tail-call" } */
> +
>   class C
>   {
>     double x;
> @@ -31,4 +35,5 @@ int main()
>     g1<int>();
>     g2<double>();
>     g3<C>();
> +  g4<int>();
>   }
>
  

On Tue, Jul 16, 2024 at 02:51:13PM -0400, Jason Merrill wrote:
> On 7/16/24 12:18 PM, Andi Kleen wrote:
> > On Tue, Jul 16, 2024 at 11:17:14AM -0400, Jason Merrill wrote:
> > > On 7/16/24 11:15 AM, Andi Kleen wrote:
> > > > > In the adjusted test it looks like the types of f and g match, so I wouldn't
> > > > > expect an error.
> > > > 
> > > > Good point! Missing the forest for the trees.
> > > > 
> > > > Anyways are the C++ patches ok with this change?
> > > 
> > > I'm still looking for a test which does error because the types are
> > > different.
> > 
> > Like this?
> 
> Where the called function returns C and the callee function does not.

In this case the attribute seems to get lost and it succeeds.

diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
index e454a6238a06..39f0ec38253d 100644
--- a/gcc/testsuite/g++.dg/musttail10.C
+++ b/gcc/testsuite/g++.dg/musttail10.C
@@ -28,12 +28,18 @@ class C
 public:
   C(double x) : x(x) {}
   ~C() { asm("":::"memory"); }
+  operator int() { return x; } 
 };
 
+template <class T>
+__attribute__((noinline, noclone, noipa))
+T g5() { [[gnu::musttail]] return f<C>(); } /* { dg-error "cannot tail-call" } */
+
 int main()
 {
   g1<int>();
   g2<double>();
   g3<C>();
   g4<int>();
+  g5<int>();
 }
  

On Tue, Jul 16, 2024 at 12:52:31PM -0700, Andi Kleen wrote:
> On Tue, Jul 16, 2024 at 02:51:13PM -0400, Jason Merrill wrote:
> > On 7/16/24 12:18 PM, Andi Kleen wrote:
> > > On Tue, Jul 16, 2024 at 11:17:14AM -0400, Jason Merrill wrote:
> > > > On 7/16/24 11:15 AM, Andi Kleen wrote:
> > > > > > In the adjusted test it looks like the types of f and g match, so I wouldn't
> > > > > > expect an error.
> > > > > 
> > > > > Good point! Missing the forest for the trees.
> > > > > 
> > > > > Anyways are the C++ patches ok with this change?
> > > > 
> > > > I'm still looking for a test which does error because the types are
> > > > different.
> > > 
> > > Like this?
> > 
> > Where the called function returns C and the callee function does not.
> 
> In this case the attribute seems to get lost and it succeeds.

This somewhat hackish patch fixes it here, to handle the case
of a TARGET_EXPR where the CALL_EXPR is in the cleanup. extract_call
bails on that.

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 3b914089a6e2..8753aa51da52 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -21124,6 +21124,10 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 		  CALL_EXPR_REVERSE_ARGS (call) = rev;
 		  if (TREE_CODE (call) == CALL_EXPR)
 		    CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
+		  else if (mtc
+			   && TREE_CODE (ret) == TARGET_EXPR
+			   && TREE_CODE (TARGET_EXPR_CLEANUP (ret)) == CALL_EXPR)
+		    CALL_EXPR_MUST_TAIL_CALL (TARGET_EXPR_CLEANUP (ret)) = mtc;
 		}
 	    if (warning_suppressed_p (t, OPT_Wpessimizing_move))
 	      /* This also suppresses -Wredundant-move.  */
  

On 7/16/24 5:55 PM, Andi Kleen wrote:
> On Tue, Jul 16, 2024 at 12:52:31PM -0700, Andi Kleen wrote:
>> On Tue, Jul 16, 2024 at 02:51:13PM -0400, Jason Merrill wrote:
>>> On 7/16/24 12:18 PM, Andi Kleen wrote:
>>>> On Tue, Jul 16, 2024 at 11:17:14AM -0400, Jason Merrill wrote:
>>>>> On 7/16/24 11:15 AM, Andi Kleen wrote:
>>>>>>> In the adjusted test it looks like the types of f and g match, so I wouldn't
>>>>>>> expect an error.
>>>>>>
>>>>>> Good point! Missing the forest for the trees.
>>>>>>
>>>>>> Anyways are the C++ patches ok with this change?
>>>>>
>>>>> I'm still looking for a test which does error because the types are
>>>>> different.
>>>>
>>>> Like this?
>>>
>>> Where the called function returns C and the callee function does not.
>>
>> In this case the attribute seems to get lost and it succeeds.
> 
> This somewhat hackish patch fixes it here, to handle the case
> of a TARGET_EXPR where the CALL_EXPR is in the cleanup. extract_call
> bails on that.

The CALL_EXPR in the cleanup is calling the destructor, that's not what 
we're trying to tail-call.

I think the problem here is that the call to f<C> is represented with an 
AGGR_INIT_EXPR instead of CALL_EXPR, so you need to handle the flag on 
that tree_code as well.

> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 3b914089a6e2..8753aa51da52 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -21124,6 +21124,10 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   		  CALL_EXPR_REVERSE_ARGS (call) = rev;
>   		  if (TREE_CODE (call) == CALL_EXPR)
>   		    CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
> +		  else if (mtc
> +			   && TREE_CODE (ret) == TARGET_EXPR
> +			   && TREE_CODE (TARGET_EXPR_CLEANUP (ret)) == CALL_EXPR)
> +		    CALL_EXPR_MUST_TAIL_CALL (TARGET_EXPR_CLEANUP (ret)) = mtc;
>   		}
>   	    if (warning_suppressed_p (t, OPT_Wpessimizing_move))
>   	      /* This also suppresses -Wredundant-move.  */
>
  

On Tue, Jul 16, 2024 at 06:06:42PM -0400, Jason Merrill wrote:
> On 7/16/24 5:55 PM, Andi Kleen wrote:
> > On Tue, Jul 16, 2024 at 12:52:31PM -0700, Andi Kleen wrote:
> > > On Tue, Jul 16, 2024 at 02:51:13PM -0400, Jason Merrill wrote:
> > > > On 7/16/24 12:18 PM, Andi Kleen wrote:
> > > > > On Tue, Jul 16, 2024 at 11:17:14AM -0400, Jason Merrill wrote:
> > > > > > On 7/16/24 11:15 AM, Andi Kleen wrote:
> > > > > > > > In the adjusted test it looks like the types of f and g match, so I wouldn't
> > > > > > > > expect an error.
> > > > > > > 
> > > > > > > Good point! Missing the forest for the trees.
> > > > > > > 
> > > > > > > Anyways are the C++ patches ok with this change?
> > > > > > 
> > > > > > I'm still looking for a test which does error because the types are
> > > > > > different.
> > > > > 
> > > > > Like this?
> > > > 
> > > > Where the called function returns C and the callee function does not.
> > > 
> > > In this case the attribute seems to get lost and it succeeds.
> > 
> > This somewhat hackish patch fixes it here, to handle the case
> > of a TARGET_EXPR where the CALL_EXPR is in the cleanup. extract_call
> > bails on that.
> 
> The CALL_EXPR in the cleanup is calling the destructor, that's not what
> we're trying to tail-call.
> 
> I think the problem here is that the call to f<C> is represented with an
> AGGR_INIT_EXPR instead of CALL_EXPR, so you need to handle the flag on that
> tree_code as well.


Okay this seems to work
(I had to adjust the test case because it now correctly errors out
on passing the class at -O0) 


diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 4bb3e9c4989b..5ec8102c1849 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4245,6 +4245,10 @@ templated_operator_saved_lookups (tree t)
 #define AGGR_INIT_FROM_THUNK_P(NODE) \
   (AGGR_INIT_EXPR_CHECK (NODE)->base.protected_flag)
 
+/* Nonzero means that the call was marked musttail.  */
+#define AGGR_INIT_EXPR_MUST_TAIL(NODE) \
+  (AGGR_INIT_EXPR_CHECK (NODE)->base.static_flag)
+
 /* AGGR_INIT_EXPR accessors.  These are equivalent to the CALL_EXPR
    accessors, except for AGGR_INIT_EXPR_SLOT (which takes the place of
    CALL_EXPR_STATIC_CHAIN).  */
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 3b914089a6e2..d668c5af6a23 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -21124,6 +21124,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 		  CALL_EXPR_REVERSE_ARGS (call) = rev;
 		  if (TREE_CODE (call) == CALL_EXPR)
 		    CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
+		  else if (TREE_CODE (call) == AGGR_INIT_EXPR)
+		    AGGR_INIT_EXPR_MUST_TAIL (call) = mtc;
 		}
 	    if (warning_suppressed_p (t, OPT_Wpessimizing_move))
 	      /* This also suppresses -Wredundant-move.  */
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index cd3df13772db..fb45974cd90f 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -4979,6 +4979,7 @@ simplify_aggr_init_expr (tree *tp)
     = CALL_EXPR_OPERATOR_SYNTAX (aggr_init_expr);
   CALL_EXPR_ORDERED_ARGS (call_expr) = CALL_EXPR_ORDERED_ARGS (aggr_init_expr);
   CALL_EXPR_REVERSE_ARGS (call_expr) = CALL_EXPR_REVERSE_ARGS (aggr_init_expr);
+  CALL_EXPR_MUST_TAIL_CALL (call_expr) = AGGR_INIT_EXPR_MUST_TAIL (aggr_init_expr);
 
   if (style == ctor)
     {
diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
index e454a6238a06..93ec32db160a 100644
--- a/gcc/testsuite/g++.dg/musttail10.C
+++ b/gcc/testsuite/g++.dg/musttail10.C
@@ -14,9 +14,11 @@ template <class T>
 __attribute__((noinline, noclone, noipa))
 T g2() { [[gnu::musttail]] return f<T>(); }
 
+#if __OPTIMIZE__ >= 1
 template <class T>
 __attribute__((noinline, noclone, noipa))
 T g3() { [[gnu::musttail]] return f<T>(); }
+#endif
 
 template <class T>
 __attribute__((noinline, noclone, noipa))
@@ -28,12 +30,20 @@ class C
 public:
   C(double x) : x(x) {}
   ~C() { asm("":::"memory"); }
+  operator int() { return x; } 
 };
 
+template <class T>
+__attribute__((noinline, noclone, noipa))
+T g5() { [[gnu::musttail]] return f<C>(); } /* { dg-error "cannot tail-call" } */
+
 int main()
 {
   g1<int>();
   g2<double>();
+#if __OPTIMIZE__ >= 1
   g3<C>();
+#endif
   g4<int>();
+  g5<int>();
 }
  

On 7/16/24 6:54 PM, Andi Kleen wrote:
> On Tue, Jul 16, 2024 at 06:06:42PM -0400, Jason Merrill wrote:
>> On 7/16/24 5:55 PM, Andi Kleen wrote:
>>> On Tue, Jul 16, 2024 at 12:52:31PM -0700, Andi Kleen wrote:
>>>> On Tue, Jul 16, 2024 at 02:51:13PM -0400, Jason Merrill wrote:
>>>>> On 7/16/24 12:18 PM, Andi Kleen wrote:
>>>>>> On Tue, Jul 16, 2024 at 11:17:14AM -0400, Jason Merrill wrote:
>>>>>>> On 7/16/24 11:15 AM, Andi Kleen wrote:
>>>>>>>>> In the adjusted test it looks like the types of f and g match, so I wouldn't
>>>>>>>>> expect an error.
>>>>>>>>
>>>>>>>> Good point! Missing the forest for the trees.
>>>>>>>>
>>>>>>>> Anyways are the C++ patches ok with this change?
>>>>>>>
>>>>>>> I'm still looking for a test which does error because the types are
>>>>>>> different.
>>>>>>
>>>>>> Like this?
>>>>>
>>>>> Where the called function returns C and the callee function does not.
>>>>
>>>> In this case the attribute seems to get lost and it succeeds.
>>>
>>> This somewhat hackish patch fixes it here, to handle the case
>>> of a TARGET_EXPR where the CALL_EXPR is in the cleanup. extract_call
>>> bails on that.
>>
>> The CALL_EXPR in the cleanup is calling the destructor, that's not what
>> we're trying to tail-call.
>>
>> I think the problem here is that the call to f<C> is represented with an
>> AGGR_INIT_EXPR instead of CALL_EXPR, so you need to handle the flag on that
>> tree_code as well.
> 
> Okay this seems to work
> (I had to adjust the test case because it now correctly errors out
> on passing the class at -O0)

Great. Does it also work in a non-template function?

> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 4bb3e9c4989b..5ec8102c1849 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -4245,6 +4245,10 @@ templated_operator_saved_lookups (tree t)
>   #define AGGR_INIT_FROM_THUNK_P(NODE) \
>     (AGGR_INIT_EXPR_CHECK (NODE)->base.protected_flag)
>   
> +/* Nonzero means that the call was marked musttail.  */
> +#define AGGR_INIT_EXPR_MUST_TAIL(NODE) \
> +  (AGGR_INIT_EXPR_CHECK (NODE)->base.static_flag)
> +
>   /* AGGR_INIT_EXPR accessors.  These are equivalent to the CALL_EXPR
>      accessors, except for AGGR_INIT_EXPR_SLOT (which takes the place of
>      CALL_EXPR_STATIC_CHAIN).  */
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 3b914089a6e2..d668c5af6a23 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -21124,6 +21124,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   		  CALL_EXPR_REVERSE_ARGS (call) = rev;
>   		  if (TREE_CODE (call) == CALL_EXPR)
>   		    CALL_EXPR_MUST_TAIL_CALL (call) = mtc;
> +		  else if (TREE_CODE (call) == AGGR_INIT_EXPR)
> +		    AGGR_INIT_EXPR_MUST_TAIL (call) = mtc;
>   		}
>   	    if (warning_suppressed_p (t, OPT_Wpessimizing_move))
>   	      /* This also suppresses -Wredundant-move.  */
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index cd3df13772db..fb45974cd90f 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -4979,6 +4979,7 @@ simplify_aggr_init_expr (tree *tp)
>       = CALL_EXPR_OPERATOR_SYNTAX (aggr_init_expr);
>     CALL_EXPR_ORDERED_ARGS (call_expr) = CALL_EXPR_ORDERED_ARGS (aggr_init_expr);
>     CALL_EXPR_REVERSE_ARGS (call_expr) = CALL_EXPR_REVERSE_ARGS (aggr_init_expr);
> +  CALL_EXPR_MUST_TAIL_CALL (call_expr) = AGGR_INIT_EXPR_MUST_TAIL (aggr_init_expr);
>   
>     if (style == ctor)
>       {
> diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
> index e454a6238a06..93ec32db160a 100644
> --- a/gcc/testsuite/g++.dg/musttail10.C
> +++ b/gcc/testsuite/g++.dg/musttail10.C
> @@ -14,9 +14,11 @@ template <class T>
>   __attribute__((noinline, noclone, noipa))
>   T g2() { [[gnu::musttail]] return f<T>(); }
>   
> +#if __OPTIMIZE__ >= 1
>   template <class T>
>   __attribute__((noinline, noclone, noipa))
>   T g3() { [[gnu::musttail]] return f<T>(); }
> +#endif
>   
>   template <class T>
>   __attribute__((noinline, noclone, noipa))
> @@ -28,12 +30,20 @@ class C
>   public:
>     C(double x) : x(x) {}
>     ~C() { asm("":::"memory"); }
> +  operator int() { return x; }
>   };
>   
> +template <class T>
> +__attribute__((noinline, noclone, noipa))
> +T g5() { [[gnu::musttail]] return f<C>(); } /* { dg-error "cannot tail-call" } */
> +
>   int main()
>   {
>     g1<int>();
>     g2<double>();
> +#if __OPTIMIZE__ >= 1
>     g3<C>();
> +#endif
>     g4<int>();
> +  g5<int>();
>   }
>
  

> Great. Does it also work in a non-template function?

Sadly it did not because there needs to be more AGGR_VIEW_EXPR handling,
as you predicted at some point. I fixed it now. Will send updated patches.

-Andi