* [PATCH] Fix "#pragma GCC pop_options" warning.
@ 2015-10-13 12:02 Dominik Vogt
2015-10-13 12:28 ` Bernd Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Dominik Vogt @ 2015-10-13 12:02 UTC (permalink / raw)
To: gcc-patches; +Cc: Andreas Krebbel, Stefan Liebler
[-- Attachment #1: Type: text/plain, Size: 777 bytes --]
When "#pragma GCC pop_options" is used on a platform without
support for "#pragma GCC target", Gcc emits a warning. As
pop_options is useful on targets without the target pragma to
restore optimizations flags, the warning should be removed.
The attached patch does that rather inelegantly by checking if the
pragma_parse hook points to the default implementation. I could't
think of a similarly terse but less clumsy way. Suggestions for a
better test are very welcome.
gcc/ChangeLog:
* c-pragma.c: Include targhooks.h.
(handle_pragma_pop_options): Do not call
default_target_option_pragma_parse to prevent its warning when using
"#pragma GCC pop_options" on platforms that do not support
"#pragma GCC target".
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
[-- Attachment #2: 0001-Fix-pragma-GCC-pop_options-warning.patch --]
[-- Type: text/x-diff, Size: 1188 bytes --]
From d149dd8b9d6c9f720809de3839f2ad5a6825f7e5 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Tue, 13 Oct 2015 12:55:21 +0100
Subject: [PATCH] Fix "#pragma GCC pop_options" warning.
---
gcc/c-family/c-pragma.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index 3c34800..b209b7b 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -38,6 +38,7 @@ along with GCC; see the file COPYING3. If not see
#include "tm_p.h" /* For REGISTER_TARGET_PRAGMAS (why is
this not a target hook?). */
#include "target.h"
+#include "targhooks.h"
#include "diagnostic.h"
#include "opts.h"
#include "plugin.h"
@@ -997,7 +998,9 @@ handle_pragma_pop_options (cpp_reader *ARG_UNUSED(dummy))
if (p->target_binary != target_option_current_node)
{
- (void) targetm.target_option.pragma_parse (NULL_TREE, p->target_binary);
+ if (targetm.target_option.pragma_parse
+ != default_target_option_pragma_parse)
+ (void) targetm.target_option.pragma_parse (NULL_TREE, p->target_binary);
target_option_current_node = p->target_binary;
}
--
2.3.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix "#pragma GCC pop_options" warning.
2015-10-13 12:02 [PATCH] Fix "#pragma GCC pop_options" warning Dominik Vogt
@ 2015-10-13 12:28 ` Bernd Schmidt
2015-10-13 13:31 ` Dominik Vogt
0 siblings, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2015-10-13 12:28 UTC (permalink / raw)
To: gcc-patches, Andreas Krebbel, Stefan Liebler
On 10/13/2015 02:02 PM, Dominik Vogt wrote:
> When "#pragma GCC pop_options" is used on a platform without
> support for "#pragma GCC target", Gcc emits a warning. As
> pop_options is useful on targets without the target pragma to
> restore optimizations flags, the warning should be removed.
>
> The attached patch does that rather inelegantly by checking if the
> pragma_parse hook points to the default implementation. I could't
> think of a similarly terse but less clumsy way. Suggestions for a
> better test are very welcome.
>
> gcc/ChangeLog:
>
> * c-pragma.c: Include targhooks.h.
> (handle_pragma_pop_options): Do not call
> default_target_option_pragma_parse to prevent its warning when using
> "#pragma GCC pop_options" on platforms that do not support
> "#pragma GCC target".
Why not just remove the code that emits the warning message? Are there
situations where the warning is justified?
A testcase would be good.
Bernd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix "#pragma GCC pop_options" warning.
2015-10-13 12:28 ` Bernd Schmidt
@ 2015-10-13 13:31 ` Dominik Vogt
2015-10-13 14:33 ` Bernd Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Dominik Vogt @ 2015-10-13 13:31 UTC (permalink / raw)
To: gcc-patches; +Cc: Andreas Krebbel, Stefan Liebler
On Tue, Oct 13, 2015 at 02:28:37PM +0200, Bernd Schmidt wrote:
> On 10/13/2015 02:02 PM, Dominik Vogt wrote:
> >When "#pragma GCC pop_options" is used on a platform without
> >support for "#pragma GCC target", Gcc emits a warning. As
> >pop_options is useful on targets without the target pragma to
> >restore optimizations flags, the warning should be removed.
> >
> >The attached patch does that rather inelegantly by checking if the
> >pragma_parse hook points to the default implementation. I could't
> >think of a similarly terse but less clumsy way. Suggestions for a
> >better test are very welcome.
>
> Why not just remove the code that emits the warning message? Are
> there situations where the warning is justified?
Removing the warning would also affect "#pragma GCC target("foo")
But then, "#pragma GCC asdfg" doesn't produce a warning either, so
what's the point warning about an undefined "target" pragma, but
not about other undefined pragmas. For me, either way to do this
is good.
By the way, the background is that Glibc used pop_options and the
warning broke building with -Werror (they have solved that in a
different way now).
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix "#pragma GCC pop_options" warning.
2015-10-13 13:31 ` Dominik Vogt
@ 2015-10-13 14:33 ` Bernd Schmidt
2015-10-13 15:03 ` Dominik Vogt
0 siblings, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2015-10-13 14:33 UTC (permalink / raw)
To: gcc-patches, Andreas Krebbel, Stefan Liebler
On 10/13/2015 03:31 PM, Dominik Vogt wrote:
> On Tue, Oct 13, 2015 at 02:28:37PM +0200, Bernd Schmidt wrote:
>> On 10/13/2015 02:02 PM, Dominik Vogt wrote:
>>> When "#pragma GCC pop_options" is used on a platform without
>>> support for "#pragma GCC target", Gcc emits a warning. As
>>> pop_options is useful on targets without the target pragma to
>>> restore optimizations flags, the warning should be removed.
>>>
>>> The attached patch does that rather inelegantly by checking if the
>>> pragma_parse hook points to the default implementation. I could't
>>> think of a similarly terse but less clumsy way. Suggestions for a
>>> better test are very welcome.
Ok, I had to go look at the code to figure out what's going on. A
suggestion for a possibly less clumsy way - recognize which pragma we're
looking at from the arguments. Looks like ix86_pragma_target_parse has a
"! args" test to determine if it has a pop, maybe the default function
could do the same. If that's insufficient, pass another argument to
identify clearly in what situation the hook is being parsed.
Bernd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix "#pragma GCC pop_options" warning.
2015-10-13 14:33 ` Bernd Schmidt
@ 2015-10-13 15:03 ` Dominik Vogt
2015-10-13 15:05 ` Bernd Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Dominik Vogt @ 2015-10-13 15:03 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 318 bytes --]
On Tue, Oct 13, 2015 at 04:33:42PM +0200, Bernd Schmidt wrote:
> Looks like
> ix86_pragma_target_parse has a "! args" test to determine if it has
> a pop, maybe the default function could do the same.
All right, this solution is way better. New patch attached.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
[-- Attachment #2: 0001-ChangeLog --]
[-- Type: text/plain, Size: 201 bytes --]
gcc/ChangeLog
* targhooks.c (default_target_option_pragma_parse): Do not warn if
called on behalf of "#pragma GCC pop_options".
gcc/testsuite/ChangeLog
* gcc.dg/pragma-pop_options-1.c: New test.
[-- Attachment #3: 0001-Remove-pragma-GCC-pop_options-warning-for-pragma-GCC.patch --]
[-- Type: text/x-diff, Size: 1543 bytes --]
From 4bb0068875e005b2f0e33bec0bd5a70b798af6e3 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Tue, 13 Oct 2015 15:54:15 +0100
Subject: [PATCH] Remove "#pragma GCC pop_options" warning for "#pragma GCC
pop_options".
---
gcc/targhooks.c | 8 ++++++--
gcc/testsuite/gcc.dg/pragma-pop_options-1.c | 7 +++++++
2 files changed, 13 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pragma-pop_options-1.c
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 7238c8f..5077ec9 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1305,8 +1305,12 @@ bool
default_target_option_pragma_parse (tree ARG_UNUSED (args),
tree ARG_UNUSED (pop_target))
{
- warning (OPT_Wpragmas,
- "#pragma GCC target is not supported for this machine");
+ /* If args is NULL the caller is handle_pragma_pop_options (). In that case,
+ emit no warning because "#pragma GCC pop_target" is valid on targets that
+ do not have the "target" pragma. */
+ if (args)
+ warning (OPT_Wpragmas,
+ "#pragma GCC target is not supported for this machine");
return false;
}
diff --git a/gcc/testsuite/gcc.dg/pragma-pop_options-1.c b/gcc/testsuite/gcc.dg/pragma-pop_options-1.c
new file mode 100644
index 0000000..4e969de
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pragma-pop_options-1.c
@@ -0,0 +1,7 @@
+/* Check warnings produced by #pragma GCC push/pop/reset_options. */
+/* { dg-do assemble } */
+
+#pragma push_options
+#pragma pop_options
+
+int foo;
--
2.3.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix "#pragma GCC pop_options" warning.
2015-10-13 15:03 ` Dominik Vogt
@ 2015-10-13 15:05 ` Bernd Schmidt
0 siblings, 0 replies; 6+ messages in thread
From: Bernd Schmidt @ 2015-10-13 15:05 UTC (permalink / raw)
To: gcc-patches, vogt
On 10/13/2015 05:03 PM, Dominik Vogt wrote:
> On Tue, Oct 13, 2015 at 04:33:42PM +0200, Bernd Schmidt wrote:
>> Looks like
>> ix86_pragma_target_parse has a "! args" test to determine if it has
>> a pop, maybe the default function could do the same.
>
> All right, this solution is way better. New patch attached.
This is ok, thanks!
Bernd
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-13 15:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-13 12:02 [PATCH] Fix "#pragma GCC pop_options" warning Dominik Vogt
2015-10-13 12:28 ` Bernd Schmidt
2015-10-13 13:31 ` Dominik Vogt
2015-10-13 14:33 ` Bernd Schmidt
2015-10-13 15:03 ` Dominik Vogt
2015-10-13 15:05 ` Bernd Schmidt
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).