From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 123027 invoked by alias); 22 Nov 2016 15:53:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 123007 invoked by uid 89); 22 Nov 2016 15:53:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=1,35, restored, solely, act X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 Nov 2016 15:53:49 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AED388F27A; Tue, 22 Nov 2016 15:53:47 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-204-19.brq.redhat.com [10.40.204.19]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uAMFrjo3015118 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 22 Nov 2016 10:53:46 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id uAMFrhar016050; Tue, 22 Nov 2016 16:53:44 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id uAMFrfSL016049; Tue, 22 Nov 2016 16:53:41 +0100 Date: Tue, 22 Nov 2016 15:53:00 -0000 From: Jakub Jelinek To: Richard Biener , Jason Merrill , "Joseph S. Myers" , Marek Polacek , Uros Bizjak Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix up handle_pragma_target (PR target/78451) Message-ID: <20161122155341.GW3541@tucnak.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg02237.txt.bz2 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 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 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