[PATCH] Fix up handle_pragma_target (PR target/78451)

[PATCH] Fix up handle_pragma_target (PR target/78451)

[Jakub Jelinek]

Hi!

#pragma GCC targets when used more than once without being
undone through #pragma GCC pop_options in between seems to act wierdly
and is the reason why sse-22a.c testcase now fails on x86_64/i686-linux.
The problem is that to some extent
#pragma GCC target ("f1", "f2,f3")
#pragma GCC target ("f4,f5", "f6")
acts as
#pragma GCC target ("f1", "f2,f3", "f4,f5", "f6")
(when computing the current set of global options e.g.), but
when a target node is being created for a function, we don't use the
current global options at the point of declaration, but instead use
current_target_pragma TREE_LIST with the current target pragma options;
that list is properly saved/restored on push_options/pop_options pragma,
but a new GCC target pragma overwrites the previous list rather than
appending to it, so to some other extent the above two pragmas act as
just #pragma GCC target ("f4,f5", "f6").
In particular, in sse-22a.c test we start with #pragma GCC target
containing huge list of ISAs, then #include <immintrin.h> header, and
there most inlines are wrapped in #pragma GCC push_options/#pragma GCC
target (someisa) and #pragma GCC pop_options, but there are some inlines
that aren't wrapped at all.  The effect of that is that those wrapped
routines get their target attribute solely from the innermost target option,
while those not wrapped ones get one from their innermost GCC target,
which is the huge list of ISAs.  I think this is undesirable, the
pragmas should stack (append to each other).  If users want to override
completely to something different, they can push_options/pop_options around
the former, or #pragma GCC target ("no-isa1,no-isa2,isa3").
Note that sse-22a.c fails because of this with -save-temps even in GCC 6.

The patch just treats these consistently as appending to current set of
options.  So if one does:
#pragma GCC push_options
#pragma GCC target ("isa1")
#pragma GCC push_options
#pragma GCC target ("isa2")
void foo () { ... }
#pragma GCC pop_options
#pragma GCC pop_options
the foo function gets both isa1 and isa2 target attributes (in that order).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-11-22  Jakub Jelinek  <jakub@redhat.com>

	PR target/78451
	* c-pragma.c (handle_pragma_target): Don't replace
	current_target_pragma, but chainon the new args to the current one.

	* gcc.target/i386/pr78451.c: New test.
	* gcc.target/i386/pr69255-1.c: Use #pragma GCC push_options
	and #pragma GCC pop_options around the first #pragma GCC target.
	* gcc.target/i386/pr69255-2.c: Likewise.
	* gcc.target/i386/pr69255-3.c: Likewise.

--- gcc/c-family/c-pragma.c.jj	2016-10-31 13:28:06.000000000 +0100
+++ gcc/c-family/c-pragma.c	2016-11-22 11:34:34.535159762 +0100
@@ -893,7 +893,7 @@ handle_pragma_target(cpp_reader *ARG_UNU
       args = nreverse (args);
 
       if (targetm.target_option.pragma_parse (args, NULL_TREE))
-	current_target_pragma = args;
+	current_target_pragma = chainon (current_target_pragma, args);
     }
 }
 
--- gcc/testsuite/gcc.target/i386/pr78451.c.jj	2016-11-22 11:57:24.743002256 +0100
+++ gcc/testsuite/gcc.target/i386/pr78451.c	2016-11-22 11:56:51.000000000 +0100
@@ -0,0 +1,35 @@
+/* PR target/78451 */
+/* { dg-options "-O2 -mno-avx512f" } */
+
+#pragma GCC push_options
+#pragma GCC target ("avx512bw")
+
+static inline int __attribute__ ((__always_inline__))
+bar (void)
+{
+  return 0;
+}
+
+#pragma GCC push_options
+#pragma GCC target ("avx512vl")
+
+int
+foo (void)
+{
+  return bar ();
+}
+
+#pragma GCC pop_options
+#pragma GCC pop_options
+
+#pragma GCC push_options
+#pragma GCC target ("avx512vl")
+#pragma GCC target ("avx512bw")
+
+int
+baz (void)
+{
+  return bar ();
+}
+
+#pragma GCC pop_options
--- gcc/testsuite/gcc.target/i386/pr69255-1.c.jj	2016-09-06 22:29:59.000000000 +0200
+++ gcc/testsuite/gcc.target/i386/pr69255-1.c	2016-11-22 16:20:32.790498858 +0100
@@ -2,7 +2,9 @@
 /* { dg-do compile } */
 /* { dg-options "-msse4 -mno-avx" } */
 
+#pragma GCC push_options
 #pragma GCC target "avx512vl"
+#pragma GCC pop_options
 #pragma GCC target "no-avx512vl"
 __attribute__ ((__vector_size__ (32))) long long a;
 __attribute__ ((__vector_size__ (16))) int b;
@@ -13,5 +15,5 @@ foo (const long long *p)
   a = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);	/* { dg-error "needs isa option -m32 -mavx512vl" } */
 }
 
-/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
-/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
+/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } 15 } */
+/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 15 } */
--- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj	2016-09-06 22:29:59.000000000 +0200
+++ gcc/testsuite/gcc.target/i386/pr69255-2.c	2016-11-22 16:20:44.760346741 +0100
@@ -2,7 +2,9 @@
 /* { dg-do compile } */
 /* { dg-options "-msse4 -mno-avx" } */
 
+#pragma GCC push_options
 #pragma GCC target "avx512vl"
+#pragma GCC pop_options
 #pragma GCC target ""
 __attribute__ ((__vector_size__ (32))) long long a;
 __attribute__ ((__vector_size__ (16))) int b;
@@ -13,5 +15,5 @@ foo (const long long *p)
   __builtin_ia32_gather3siv4di (a, p, b, 1, 1);		/* { dg-error "needs isa option -m32 -mavx512vl" } */
 }
 
-/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
-/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
+/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } 15 } */
+/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 15 } */
--- gcc/testsuite/gcc.target/i386/pr69255-3.c.jj	2016-09-06 22:29:59.000000000 +0200
+++ gcc/testsuite/gcc.target/i386/pr69255-3.c	2016-11-22 16:21:54.054466126 +0100
@@ -2,7 +2,9 @@
 /* { dg-do compile } */
 /* { dg-options "-msse4 -mno-avx" } */
 
+#pragma GCC push_options
 #pragma GCC target "avx512vl"
+#pragma GCC pop_options
 #pragma GCC target ""
 __attribute__ ((__vector_size__ (32))) long long a;
 __attribute__ ((__vector_size__ (16))) int b;
@@ -13,5 +15,5 @@ foo (const long long *p, __attribute__ (
   *q = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);	/* { dg-error "needs isa option -m32 -mavx512vl" } */
 }
 
-/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
-/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
+/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } 15 } */
+/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 15 } */


	Jakub

Re: [PATCH] Fix up handle_pragma_target (PR target/78451)

[Joseph Myers]

On Tue, 22 Nov 2016, Jakub Jelinek wrote:

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2016-11-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/78451
> 	* c-pragma.c (handle_pragma_target): Don't replace
> 	current_target_pragma, but chainon the new args to the current one.
> 
> 	* gcc.target/i386/pr78451.c: New test.
> 	* gcc.target/i386/pr69255-1.c: Use #pragma GCC push_options
> 	and #pragma GCC pop_options around the first #pragma GCC target.
> 	* gcc.target/i386/pr69255-2.c: Likewise.
> 	* gcc.target/i386/pr69255-3.c: Likewise.

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com