public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks
@ 2024-07-25 22:55 Andi Kleen
  2024-07-25 22:55 ` [PATCH v1 2/2] PR116019: Improve tail call error message Andi Kleen
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Andi Kleen @ 2024-07-25 22:55 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

From: Andi Kleen <ak@gcc.gnu.org>

- Run the target_effective tail_call checks without optimization to
match the actual test cases.
- Add an extra check for external tail calls to handle targets like
powerpc that cannot tail call between different object files.
This one will also cover templates.

gcc/testsuite/ChangeLog:

	PR testsuite/116080
	* g++.dg/musttail10.C: Use external tail call target check.
	* g++.dg/musttail6.C: Dito.
	* lib/target-supports.exp: Add external_tail_call. Disable
	optimization for tail call checks.
---
 gcc/testsuite/g++.dg/musttail10.C     |  2 +-
 gcc/testsuite/g++.dg/musttail6.C      |  2 +-
 gcc/testsuite/lib/target-supports.exp | 14 +++++++++++---
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
index ff7fcc7d8755..bd75affa2220 100644
--- a/gcc/testsuite/g++.dg/musttail10.C
+++ b/gcc/testsuite/g++.dg/musttail10.C
@@ -8,7 +8,7 @@ double g() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-cal
 
 template <class T>
 __attribute__((noinline, noclone, noipa))
-T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */
+T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target { external_tail_call } } } */
 
 template <class T>
 __attribute__((noinline, noclone, noipa))
diff --git a/gcc/testsuite/g++.dg/musttail6.C b/gcc/testsuite/g++.dg/musttail6.C
index 5c6f69407ddb..81f6d9f3ca77 100644
--- a/gcc/testsuite/g++.dg/musttail6.C
+++ b/gcc/testsuite/g++.dg/musttail6.C
@@ -1,6 +1,6 @@
 /* { dg-do compile { target { struct_tail_call } } } */
+/* { dg-require-effective-target external_tail_call } */
 /* A lot of architectures will not build this due to PR115606 and PR115607 */
-/* { dg-skip-if "powerpc does not support sibcall to templates" { powerpc*-*-* } } */
 /* { dg-options "-std=gnu++11" } */
 /* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
 
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index d368251ef9a4..0a3946e82d4b 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -12741,7 +12741,15 @@ proc check_effective_target_tail_call { } {
     return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
 	__attribute__((__noipa__)) void foo (void) { }
 	__attribute__((__noipa__)) void bar (void) { foo(); }
-    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
+    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
+}
+
+# Return 1 if the target can perform tail-calls for externals
+proc check_effective_target_external_tail_call { } {
+    return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
+	extern __attribute__((__noipa__)) void foo (void);
+	__attribute__((__noipa__)) void bar (void) { foo(); }
+    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
 }
 
 # Return 1 if the target can perform tail-call optimizations for structures
@@ -12751,9 +12759,9 @@ proc check_effective_target_struct_tail_call { } {
     return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
 	// C++
 	struct foo { int a, b; };
-	__attribute__((__noipa__)) struct foo foo (void) { return {}; }
+	extern __attribute__((__noipa__)) struct foo foo (void);
 	__attribute__((__noipa__)) struct foo bar (void) { return foo(); }
-    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
+    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
 }
 
 # Return 1 if the target's calling sequence or its ABI
-- 
2.45.2


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

* [PATCH v1 2/2] PR116019: Improve tail call error message
  2024-07-25 22:55 [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Andi Kleen
@ 2024-07-25 22:55 ` Andi Kleen
  2024-07-26 11:33   ` Richard Biener
  2024-07-25 23:08 ` [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Sam James
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Andi Kleen @ 2024-07-25 22:55 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

From: Andi Kleen <ak@gcc.gnu.org>

The "tail call must be the same type" message is common on some
targets with C++, or without optimization. It is generated
when gcc believes there is an access of the return value
after the call. However usually it does not actually corespond
to a type mismatch, but can be caused for other reasons.

Make it slightly more vague to be less misleading.

gcc/ChangeLog:

	PR c++/116019
	* tree-tailcall.cc (find_tail_calls): Change tail call
	error message.
---
 gcc/tree-tailcall.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc
index a68079d4f507..1901b1a13f99 100644
--- a/gcc/tree-tailcall.cc
+++ b/gcc/tree-tailcall.cc
@@ -632,7 +632,7 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail,
       && may_be_aliased (result_decl)
       && ref_maybe_used_by_stmt_p (call, result_decl, false))
     {
-      maybe_error_musttail (call, _("tail call must be same type"));
+      maybe_error_musttail (call, _("return value used after call"));
       return;
     }
 
-- 
2.45.2


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

* Re: [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks
  2024-07-25 22:55 [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Andi Kleen
  2024-07-25 22:55 ` [PATCH v1 2/2] PR116019: Improve tail call error message Andi Kleen
@ 2024-07-25 23:08 ` Sam James
  2024-07-25 23:14   ` Andrew Pinski
  2024-07-26 11:34 ` Richard Biener
  2024-07-29 10:22 ` Thomas Schwinge
  3 siblings, 1 reply; 9+ messages in thread
From: Sam James @ 2024-07-25 23:08 UTC (permalink / raw)
  To: Andi Kleen; +Cc: gcc-patches, Andi Kleen

[-- Attachment #1: Type: text/plain, Size: 4128 bytes --]

Andi Kleen <ak@linux.intel.com> writes:

> From: Andi Kleen <ak@gcc.gnu.org>
>
> - Run the target_effective tail_call checks without optimization to
> match the actual test cases.
> - Add an extra check for external tail calls to handle targets like
> powerpc that cannot tail call between different object files.
> This one will also cover templates.

Two trivial comments below.

>
> gcc/testsuite/ChangeLog:
>
> 	PR testsuite/116080
> 	* g++.dg/musttail10.C: Use external tail call target check.
> 	* g++.dg/musttail6.C: Dito.

s/Dito/Ditto/

> 	* lib/target-supports.exp: Add external_tail_call. Disable
> 	optimization for tail call checks.
> ---
>  gcc/testsuite/g++.dg/musttail10.C     |  2 +-
>  gcc/testsuite/g++.dg/musttail6.C      |  2 +-
>  gcc/testsuite/lib/target-supports.exp | 14 +++++++++++---
>  3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
> index ff7fcc7d8755..bd75affa2220 100644
> --- a/gcc/testsuite/g++.dg/musttail10.C
> +++ b/gcc/testsuite/g++.dg/musttail10.C
> @@ -8,7 +8,7 @@ double g() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-cal
>  
>  template <class T>
>  __attribute__((noinline, noclone, noipa))
> -T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */
> +T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target { external_tail_call } } } */
>  
>  template <class T>
>  __attribute__((noinline, noclone, noipa))
> diff --git a/gcc/testsuite/g++.dg/musttail6.C b/gcc/testsuite/g++.dg/musttail6.C
> index 5c6f69407ddb..81f6d9f3ca77 100644
> --- a/gcc/testsuite/g++.dg/musttail6.C
> +++ b/gcc/testsuite/g++.dg/musttail6.C
> @@ -1,6 +1,6 @@
>  /* { dg-do compile { target { struct_tail_call } } } */
> +/* { dg-require-effective-target external_tail_call } */
>  /* A lot of architectures will not build this due to PR115606 and PR115607 */
> -/* { dg-skip-if "powerpc does not support sibcall to templates" { powerpc*-*-* } } */
>  /* { dg-options "-std=gnu++11" } */
>  /* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
>  
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index d368251ef9a4..0a3946e82d4b 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -12741,7 +12741,15 @@ proc check_effective_target_tail_call { } {
>      return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
>  	__attribute__((__noipa__)) void foo (void) { }
>  	__attribute__((__noipa__)) void bar (void) { foo(); }
> -    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +}
> +
> +# Return 1 if the target can perform tail-calls for externals
> +proc check_effective_target_external_tail_call { } {
> +    return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
> +	extern __attribute__((__noipa__)) void foo (void);
> +	__attribute__((__noipa__)) void bar (void) { foo(); }

There's far more instances of noipa in the testsuite than __noipa__.

> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
>  }
>  
>  # Return 1 if the target can perform tail-call optimizations for structures
> @@ -12751,9 +12759,9 @@ proc check_effective_target_struct_tail_call { } {
>      return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
>  	// C++
>  	struct foo { int a, b; };
> -	__attribute__((__noipa__)) struct foo foo (void) { return {}; }
> +	extern __attribute__((__noipa__)) struct foo foo (void);
>  	__attribute__((__noipa__)) struct foo bar (void) { return foo(); }
> -    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
>  }
>  
>  # Return 1 if the target's calling sequence or its ABI

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

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

* Re: [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks
  2024-07-25 23:08 ` [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Sam James
@ 2024-07-25 23:14   ` Andrew Pinski
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Pinski @ 2024-07-25 23:14 UTC (permalink / raw)
  To: Sam James; +Cc: Andi Kleen, gcc-patches, Andi Kleen

On Thu, Jul 25, 2024 at 4:09 PM Sam James <sam@gentoo.org> wrote:
>
> Andi Kleen <ak@linux.intel.com> writes:
>
> > From: Andi Kleen <ak@gcc.gnu.org>
> >
> > - Run the target_effective tail_call checks without optimization to
> > match the actual test cases.
> > - Add an extra check for external tail calls to handle targets like
> > powerpc that cannot tail call between different object files.
> > This one will also cover templates.
>
> Two trivial comments below.
>
> >
> > gcc/testsuite/ChangeLog:
> >
> >       PR testsuite/116080
> >       * g++.dg/musttail10.C: Use external tail call target check.
> >       * g++.dg/musttail6.C: Dito.
>
> s/Dito/Ditto/

One extra nit, It is much (~5x) more common to use "Likewise" rather
than Ditto in GCC's changelogs.
[apinski@xeond2 gcc]$ git grep Ditto *{,/}ChangeLog*|wc -l
41565
[apinski@xeond2 gcc]$ git grep Likewise *{,/}ChangeLog*|wc -l
196587

Thanks,
Andrew

>
> >       * lib/target-supports.exp: Add external_tail_call. Disable
> >       optimization for tail call checks.
> > ---
> >  gcc/testsuite/g++.dg/musttail10.C     |  2 +-
> >  gcc/testsuite/g++.dg/musttail6.C      |  2 +-
> >  gcc/testsuite/lib/target-supports.exp | 14 +++++++++++---
> >  3 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
> > index ff7fcc7d8755..bd75affa2220 100644
> > --- a/gcc/testsuite/g++.dg/musttail10.C
> > +++ b/gcc/testsuite/g++.dg/musttail10.C
> > @@ -8,7 +8,7 @@ double g() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-cal
> >
> >  template <class T>
> >  __attribute__((noinline, noclone, noipa))
> > -T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */
> > +T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target { external_tail_call } } } */
> >
> >  template <class T>
> >  __attribute__((noinline, noclone, noipa))
> > diff --git a/gcc/testsuite/g++.dg/musttail6.C b/gcc/testsuite/g++.dg/musttail6.C
> > index 5c6f69407ddb..81f6d9f3ca77 100644
> > --- a/gcc/testsuite/g++.dg/musttail6.C
> > +++ b/gcc/testsuite/g++.dg/musttail6.C
> > @@ -1,6 +1,6 @@
> >  /* { dg-do compile { target { struct_tail_call } } } */
> > +/* { dg-require-effective-target external_tail_call } */
> >  /* A lot of architectures will not build this due to PR115606 and PR115607 */
> > -/* { dg-skip-if "powerpc does not support sibcall to templates" { powerpc*-*-* } } */
> >  /* { dg-options "-std=gnu++11" } */
> >  /* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
> >
> > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> > index d368251ef9a4..0a3946e82d4b 100644
> > --- a/gcc/testsuite/lib/target-supports.exp
> > +++ b/gcc/testsuite/lib/target-supports.exp
> > @@ -12741,7 +12741,15 @@ proc check_effective_target_tail_call { } {
> >      return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
> >       __attribute__((__noipa__)) void foo (void) { }
> >       __attribute__((__noipa__)) void bar (void) { foo(); }
> > -    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> > +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> > +}
> > +
> > +# Return 1 if the target can perform tail-calls for externals
> > +proc check_effective_target_external_tail_call { } {
> > +    return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
> > +     extern __attribute__((__noipa__)) void foo (void);
> > +     __attribute__((__noipa__)) void bar (void) { foo(); }
>
> There's far more instances of noipa in the testsuite than __noipa__.
>
> > +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> >  }
> >
> >  # Return 1 if the target can perform tail-call optimizations for structures
> > @@ -12751,9 +12759,9 @@ proc check_effective_target_struct_tail_call { } {
> >      return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
> >       // C++
> >       struct foo { int a, b; };
> > -     __attribute__((__noipa__)) struct foo foo (void) { return {}; }
> > +     extern __attribute__((__noipa__)) struct foo foo (void);
> >       __attribute__((__noipa__)) struct foo bar (void) { return foo(); }
> > -    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> > +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> >  }
> >
> >  # Return 1 if the target's calling sequence or its ABI

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

* Re: [PATCH v1 2/2] PR116019: Improve tail call error message
  2024-07-25 22:55 ` [PATCH v1 2/2] PR116019: Improve tail call error message Andi Kleen
@ 2024-07-26 11:33   ` Richard Biener
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Biener @ 2024-07-26 11:33 UTC (permalink / raw)
  To: Andi Kleen; +Cc: gcc-patches, Andi Kleen

On Fri, Jul 26, 2024 at 12:55 AM Andi Kleen <ak@linux.intel.com> wrote:
>
> From: Andi Kleen <ak@gcc.gnu.org>
>
> The "tail call must be the same type" message is common on some
> targets with C++, or without optimization. It is generated
> when gcc believes there is an access of the return value
> after the call. However usually it does not actually corespond
> to a type mismatch, but can be caused for other reasons.
>
> Make it slightly more vague to be less misleading.

OK.

> gcc/ChangeLog:
>
>         PR c++/116019
>         * tree-tailcall.cc (find_tail_calls): Change tail call
>         error message.
> ---
>  gcc/tree-tailcall.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc
> index a68079d4f507..1901b1a13f99 100644
> --- a/gcc/tree-tailcall.cc
> +++ b/gcc/tree-tailcall.cc
> @@ -632,7 +632,7 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail,
>        && may_be_aliased (result_decl)
>        && ref_maybe_used_by_stmt_p (call, result_decl, false))
>      {
> -      maybe_error_musttail (call, _("tail call must be same type"));
> +      maybe_error_musttail (call, _("return value used after call"));
>        return;
>      }
>
> --
> 2.45.2
>

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

* Re: [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks
  2024-07-25 22:55 [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Andi Kleen
  2024-07-25 22:55 ` [PATCH v1 2/2] PR116019: Improve tail call error message Andi Kleen
  2024-07-25 23:08 ` [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Sam James
@ 2024-07-26 11:34 ` Richard Biener
  2024-07-29 10:22 ` Thomas Schwinge
  3 siblings, 0 replies; 9+ messages in thread
From: Richard Biener @ 2024-07-26 11:34 UTC (permalink / raw)
  To: Andi Kleen; +Cc: gcc-patches, Andi Kleen

On Fri, Jul 26, 2024 at 12:55 AM Andi Kleen <ak@linux.intel.com> wrote:
>
> From: Andi Kleen <ak@gcc.gnu.org>
>
> - Run the target_effective tail_call checks without optimization to
> match the actual test cases.
> - Add an extra check for external tail calls to handle targets like
> powerpc that cannot tail call between different object files.
> This one will also cover templates.

OK.

Thanks,
Richard.

> gcc/testsuite/ChangeLog:
>
>         PR testsuite/116080
>         * g++.dg/musttail10.C: Use external tail call target check.
>         * g++.dg/musttail6.C: Dito.
>         * lib/target-supports.exp: Add external_tail_call. Disable
>         optimization for tail call checks.
> ---
>  gcc/testsuite/g++.dg/musttail10.C     |  2 +-
>  gcc/testsuite/g++.dg/musttail6.C      |  2 +-
>  gcc/testsuite/lib/target-supports.exp | 14 +++++++++++---
>  3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/testsuite/g++.dg/musttail10.C b/gcc/testsuite/g++.dg/musttail10.C
> index ff7fcc7d8755..bd75affa2220 100644
> --- a/gcc/testsuite/g++.dg/musttail10.C
> +++ b/gcc/testsuite/g++.dg/musttail10.C
> @@ -8,7 +8,7 @@ double g() { [[gnu::musttail]] return f<int>(); } /* { dg-error "cannot tail-cal
>
>  template <class T>
>  __attribute__((noinline, noclone, noipa))
> -T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target powerpc*-*-* } } */
> +T g1() { [[gnu::musttail]] return f<T>(); } /* { dg-error "target is not able" "" { target { external_tail_call } } } */
>
>  template <class T>
>  __attribute__((noinline, noclone, noipa))
> diff --git a/gcc/testsuite/g++.dg/musttail6.C b/gcc/testsuite/g++.dg/musttail6.C
> index 5c6f69407ddb..81f6d9f3ca77 100644
> --- a/gcc/testsuite/g++.dg/musttail6.C
> +++ b/gcc/testsuite/g++.dg/musttail6.C
> @@ -1,6 +1,6 @@
>  /* { dg-do compile { target { struct_tail_call } } } */
> +/* { dg-require-effective-target external_tail_call } */
>  /* A lot of architectures will not build this due to PR115606 and PR115607 */
> -/* { dg-skip-if "powerpc does not support sibcall to templates" { powerpc*-*-* } } */
>  /* { dg-options "-std=gnu++11" } */
>  /* { dg-additional-options "-fdelayed-branch" { target sparc*-*-* } } */
>
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index d368251ef9a4..0a3946e82d4b 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -12741,7 +12741,15 @@ proc check_effective_target_tail_call { } {
>      return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
>         __attribute__((__noipa__)) void foo (void) { }
>         __attribute__((__noipa__)) void bar (void) { foo(); }
> -    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +}
> +
> +# Return 1 if the target can perform tail-calls for externals
> +proc check_effective_target_external_tail_call { } {
> +    return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
> +       extern __attribute__((__noipa__)) void foo (void);
> +       __attribute__((__noipa__)) void bar (void) { foo(); }
> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
>  }
>
>  # Return 1 if the target can perform tail-call optimizations for structures
> @@ -12751,9 +12759,9 @@ proc check_effective_target_struct_tail_call { } {
>      return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
>         // C++
>         struct foo { int a, b; };
> -       __attribute__((__noipa__)) struct foo foo (void) { return {}; }
> +       extern __attribute__((__noipa__)) struct foo foo (void);
>         __attribute__((__noipa__)) struct foo bar (void) { return foo(); }
> -    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
>  }
>
>  # Return 1 if the target's calling sequence or its ABI
> --
> 2.45.2
>

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

* Re: [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks
  2024-07-25 22:55 [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Andi Kleen
                   ` (2 preceding siblings ...)
  2024-07-26 11:34 ` Richard Biener
@ 2024-07-29 10:22 ` Thomas Schwinge
  2024-07-29 16:03   ` Andi Kleen
  3 siblings, 1 reply; 9+ messages in thread
From: Thomas Schwinge @ 2024-07-29 10:22 UTC (permalink / raw)
  To: Andi Kleen; +Cc: gcc-patches, Andi Kleen

Hi Andi!

I'm lacking all possible context here, but I noticed:

On 2024-07-25T15:55:01-0700, Andi Kleen <ak@linux.intel.com> wrote:
> - Run the target_effective tail_call checks without optimization to
> match the actual test cases.

> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -12741,7 +12741,15 @@ proc check_effective_target_tail_call { } {
>      return [check_no_messages_and_pattern tail_call ",SIBCALL" rtl-expand {
>  	__attribute__((__noipa__)) void foo (void) { }
>  	__attribute__((__noipa__)) void bar (void) { foo(); }
> -    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +}

> +proc check_effective_target_external_tail_call { } {
> +    [...]
> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
>  }

> @@ -12751,9 +12759,9 @@ proc check_effective_target_struct_tail_call { } {
>     [...]
> -    } {-O2 -fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
> +    } {-fdump-rtl-expand-all}] ;# The "SIBCALL" note requires a detailed dump.
>  }

..., that means that a number of the new test cases are UNSUPPORTED, for
example, x86_64 GNU/Linux:

    +UNSUPPORTED: c-c++-common/musttail1.c  -Wc++-compat 
    +UNSUPPORTED: c-c++-common/musttail12.c  -Wc++-compat 
    +PASS: c-c++-common/musttail13.c  -Wc++-compat   (test for errors, line 4)
    +PASS: c-c++-common/musttail13.c  -Wc++-compat  (test for excess errors)
    +UNSUPPORTED: c-c++-common/musttail2.c  -Wc++-compat 
    +UNSUPPORTED: c-c++-common/musttail3.c  -Wc++-compat 
    +UNSUPPORTED: c-c++-common/musttail4.c  -Wc++-compat 
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for errors, line 17)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 10)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 11)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 12)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 24)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 25)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 26)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 5)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 6)
    +PASS: c-c++-common/musttail5.c  -Wc++-compat  (test for excess errors)
    +UNSUPPORTED: c-c++-common/musttail7.c  -Wc++-compat 
    +UNSUPPORTED: c-c++-common/musttail8.c  -Wc++-compat 

(Similarly for their C++ testing.)

    +UNSUPPORTED: g++.dg/musttail10.C  
    +UNSUPPORTED: g++.dg/musttail11.C  
    +UNSUPPORTED: g++.dg/musttail6.C  
    +UNSUPPORTED: g++.dg/musttail9.C  

..., and even a few existing test cases "regress" from PASS to
UNSUPPORTED:

    [-PASS:-]{+UNSUPPORTED:+} gcc.dg/plugin/must-tail-call-1.c -fplugin=./must_tail_call_plugin.so[-(test for excess errors)-]
    [-PASS:-]{+UNSUPPORTED:+} gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so[-(test for errors, line 18)-]
    [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so  (test for errors, line 33)-]
    [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so  (test for errors, line 40)-]
    [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so  (test for errors, line 49)-]
    [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so  (test for errors, line 58)-]
    [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so (test for excess errors)-]

Similarly for ppc64le GNU/Linux.

Is that intentional?


Grüße
 Thomas

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

* Re: [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks
  2024-07-29 10:22 ` Thomas Schwinge
@ 2024-07-29 16:03   ` Andi Kleen
  2024-07-29 17:14     ` Andi Kleen
  0 siblings, 1 reply; 9+ messages in thread
From: Andi Kleen @ 2024-07-29 16:03 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc-patches, Andi Kleen

> ..., that means that a number of the new test cases are UNSUPPORTED, for
> example, x86_64 GNU/Linux:
> 
>     +UNSUPPORTED: c-c++-common/musttail1.c  -Wc++-compat 
>     +UNSUPPORTED: c-c++-common/musttail12.c  -Wc++-compat 
>     +PASS: c-c++-common/musttail13.c  -Wc++-compat   (test for errors, line 4)
>     +PASS: c-c++-common/musttail13.c  -Wc++-compat  (test for excess errors)
>     +UNSUPPORTED: c-c++-common/musttail2.c  -Wc++-compat 
>     +UNSUPPORTED: c-c++-common/musttail3.c  -Wc++-compat 
>     +UNSUPPORTED: c-c++-common/musttail4.c  -Wc++-compat 
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for errors, line 17)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 10)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 11)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 12)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 24)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 25)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 26)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 5)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat   (test for warnings, line 6)
>     +PASS: c-c++-common/musttail5.c  -Wc++-compat  (test for excess errors)
>     +UNSUPPORTED: c-c++-common/musttail7.c  -Wc++-compat 
>     +UNSUPPORTED: c-c++-common/musttail8.c  -Wc++-compat 
> 
> (Similarly for their C++ testing.)
> 
>     +UNSUPPORTED: g++.dg/musttail10.C  
>     +UNSUPPORTED: g++.dg/musttail11.C  
>     +UNSUPPORTED: g++.dg/musttail6.C  
>     +UNSUPPORTED: g++.dg/musttail9.C  
> 
> ..., and even a few existing test cases "regress" from PASS to
> UNSUPPORTED:
> 
>     [-PASS:-]{+UNSUPPORTED:+} gcc.dg/plugin/must-tail-call-1.c -fplugin=./must_tail_call_plugin.so[-(test for excess errors)-]
>     [-PASS:-]{+UNSUPPORTED:+} gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so[-(test for errors, line 18)-]
>     [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so  (test for errors, line 33)-]
>     [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so  (test for errors, line 40)-]
>     [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so  (test for errors, line 49)-]
>     [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so  (test for errors, line 58)-]
>     [-PASS: gcc.dg/plugin/must-tail-call-2.c -fplugin=./must_tail_call_plugin.so (test for excess errors)-]
> 
> Similarly for ppc64le GNU/Linux.
> 
> Is that intentional?

Thanks.  I will take a look. At least on x86_64-linux everything should
be supported. On powerpc and ARM I expect some unsupported. 

But the previous test cases shouldn't have changed. Maybe we need
more tail_call dejagnu tests that also enable -O2. 

The whole area is unfortunately somewhat of a mine field because of
lots of varying restrictions on tail calls, both with frontends
and targets.

-Andi

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

* Re: [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks
  2024-07-29 16:03   ` Andi Kleen
@ 2024-07-29 17:14     ` Andi Kleen
  0 siblings, 0 replies; 9+ messages in thread
From: Andi Kleen @ 2024-07-29 17:14 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Thomas Schwinge, gcc-patches, Andi Kleen



I'm going to revert the patch for now. There are two problems:

- The new tests don't have a unique name so the caching confuses 
the results.
- To test with -O2 we need explicit musttail checks because tail call doesn't
run with -O0 w/o musttail.


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

end of thread, other threads:[~2024-07-29 17:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-25 22:55 [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Andi Kleen
2024-07-25 22:55 ` [PATCH v1 2/2] PR116019: Improve tail call error message Andi Kleen
2024-07-26 11:33   ` Richard Biener
2024-07-25 23:08 ` [PATCH v1 1/2] PR116080: Fix tail call dejagnu checks Sam James
2024-07-25 23:14   ` Andrew Pinski
2024-07-26 11:34 ` Richard Biener
2024-07-29 10:22 ` Thomas Schwinge
2024-07-29 16:03   ` Andi Kleen
2024-07-29 17:14     ` 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).