* [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
@ 2016-03-04 17:30 Marek Polacek
2016-03-04 17:41 ` Jakub Jelinek
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2016-03-04 17:30 UTC (permalink / raw)
To: GCC Patches; +Cc: Joseph Myers
This is not a regression but I thought I'd post this anyway. Martin reported
that we generate -Wunused-value warnings on the attached testcase, which
arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since
we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g.
for __atomic_load/store/compare.)
Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7?
2016-03-04 Marek Polacek <polacek@redhat.com>
PR c/69407
* c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
operations.
* gcc.dg/atomic-op-6.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 965cf49..25afa9c 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -11443,6 +11443,10 @@ resolve_overloaded_builtin (location_t loc, tree function,
&& orig_code != BUILT_IN_ATOMIC_STORE_N)
result = sync_resolve_return (first_param, result, orig_format);
+ if (fetch_op)
+ /* Prevent -Wunused-value warning. */
+ TREE_USED (result) = true;
+
/* If new_return is set, assign function to that expr and cast the
result to void since the generic interface returned void. */
if (new_return)
diff --git gcc/testsuite/gcc.dg/atomic-op-6.c gcc/testsuite/gcc.dg/atomic-op-6.c
index e69de29..c8d93a4 100644
--- gcc/testsuite/gcc.dg/atomic-op-6.c
+++ gcc/testsuite/gcc.dg/atomic-op-6.c
@@ -0,0 +1,11 @@
+/* Test we don't generate bogus warnings. */
+/* PR c/69407 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+void
+foo (int *p, int a)
+{
+ __atomic_fetch_add (&p, a, 0);
+ __atomic_add_fetch (&p, a, 0);
+}
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-04 17:30 [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407) Marek Polacek
@ 2016-03-04 17:41 ` Jakub Jelinek
2016-03-04 18:03 ` Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2016-03-04 17:41 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers
On Fri, Mar 04, 2016 at 06:30:40PM +0100, Marek Polacek wrote:
> This is not a regression but I thought I'd post this anyway. Martin reported
> that we generate -Wunused-value warnings on the attached testcase, which
> arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since
> we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g.
> for __atomic_load/store/compare.)
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7?
I'm ok with it for gcc6.
> diff --git gcc/testsuite/gcc.dg/atomic-op-6.c gcc/testsuite/gcc.dg/atomic-op-6.c
> index e69de29..c8d93a4 100644
> --- gcc/testsuite/gcc.dg/atomic-op-6.c
> +++ gcc/testsuite/gcc.dg/atomic-op-6.c
> @@ -0,0 +1,11 @@
> +/* Test we don't generate bogus warnings. */
> +/* PR c/69407 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall -Wextra" } */
> +
> +void
> +foo (int *p, int a)
> +{
> + __atomic_fetch_add (&p, a, 0);
> + __atomic_add_fetch (&p, a, 0);
> +}
But IMHO you should add dg-bogus directives here.
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-04 17:41 ` Jakub Jelinek
@ 2016-03-04 18:03 ` Marek Polacek
2016-03-04 18:20 ` Jakub Jelinek
2016-03-14 11:48 ` Marek Polacek
0 siblings, 2 replies; 11+ messages in thread
From: Marek Polacek @ 2016-03-04 18:03 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: GCC Patches, Joseph Myers
On Fri, Mar 04, 2016 at 06:41:26PM +0100, Jakub Jelinek wrote:
> I'm ok with it for gcc6.
Cool.
> But IMHO you should add dg-bogus directives here.
Ok, version with dg-bogus:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-03-04 Marek Polacek <polacek@redhat.com>
PR c/69407
* c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
operations.
* gcc.dg/atomic-op-6.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 965cf49..25afa9c 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -11443,6 +11443,10 @@ resolve_overloaded_builtin (location_t loc, tree function,
&& orig_code != BUILT_IN_ATOMIC_STORE_N)
result = sync_resolve_return (first_param, result, orig_format);
+ if (fetch_op)
+ /* Prevent -Wunused-value warning. */
+ TREE_USED (result) = true;
+
/* If new_return is set, assign function to that expr and cast the
result to void since the generic interface returned void. */
if (new_return)
diff --git gcc/testsuite/gcc.dg/atomic-op-6.c gcc/testsuite/gcc.dg/atomic-op-6.c
index e69de29..f88c293 100644
--- gcc/testsuite/gcc.dg/atomic-op-6.c
+++ gcc/testsuite/gcc.dg/atomic-op-6.c
@@ -0,0 +1,11 @@
+/* Test we don't generate bogus warnings. */
+/* PR c/69407 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+void
+foo (int *p, int a)
+{
+ __atomic_fetch_add (&p, a, 0); /* { dg-bogus "value computed is not used" } */
+ __atomic_add_fetch (&p, a, 0); /* { dg-bogus "value computed is not used" } */
+}
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-04 18:03 ` Marek Polacek
@ 2016-03-04 18:20 ` Jakub Jelinek
2016-03-07 13:33 ` Marek Polacek
2016-03-14 11:48 ` Marek Polacek
1 sibling, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2016-03-04 18:20 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers
On Fri, Mar 04, 2016 at 07:03:09PM +0100, Marek Polacek wrote:
> Ok, version with dg-bogus:
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-03-04 Marek Polacek <polacek@redhat.com>
>
> PR c/69407
> * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
> operations.
>
> * gcc.dg/atomic-op-6.c: New test.
> diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
> index 965cf49..25afa9c 100644
> --- gcc/c-family/c-common.c
> +++ gcc/c-family/c-common.c
> @@ -11443,6 +11443,10 @@ resolve_overloaded_builtin (location_t loc, tree function,
> && orig_code != BUILT_IN_ATOMIC_STORE_N)
> result = sync_resolve_return (first_param, result, orig_format);
>
> + if (fetch_op)
> + /* Prevent -Wunused-value warning. */
> + TREE_USED (result) = true;
> +
Can't sync_resolve_return return error_mark_node?
If it could, perhaps it would be saver to move this a few lines above the
sync_resolve_return call, where we've already checked that result is not
error_mark_node.
Otherwise LGTM.
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-04 18:20 ` Jakub Jelinek
@ 2016-03-07 13:33 ` Marek Polacek
0 siblings, 0 replies; 11+ messages in thread
From: Marek Polacek @ 2016-03-07 13:33 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: GCC Patches, Joseph Myers
On Fri, Mar 04, 2016 at 07:19:56PM +0100, Jakub Jelinek wrote:
> > --- gcc/c-family/c-common.c
> > +++ gcc/c-family/c-common.c
> > @@ -11443,6 +11443,10 @@ resolve_overloaded_builtin (location_t loc, tree function,
> > && orig_code != BUILT_IN_ATOMIC_STORE_N)
> > result = sync_resolve_return (first_param, result, orig_format);
> >
> > + if (fetch_op)
> > + /* Prevent -Wunused-value warning. */
> > + TREE_USED (result) = true;
> > +
>
> Can't sync_resolve_return return error_mark_node?
> If it could, perhaps it would be saver to move this a few lines above the
> sync_resolve_return call, where we've already checked that result is not
> error_mark_node.
That seemed right but now I remember why I put setting TREE_USED here. Setting
TREE_USED on the result of build_function_call_vec (a CALL_EXPR) wouldn't
suppress the warning, it needs to be set on the result of sync_resolve_return
(a CONVERT_EXPR). Setting TREE_USED on error_mark_node doesn't ICE so I didn't
check for that as I think it should be harmless.
So I'm keeping the patch as-is, please let me know you still want to see some
adjustments.
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-04 18:03 ` Marek Polacek
2016-03-04 18:20 ` Jakub Jelinek
@ 2016-03-14 11:48 ` Marek Polacek
2016-03-17 17:24 ` Jeff Law
1 sibling, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2016-03-14 11:48 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: GCC Patches, Joseph Myers
Ping.
On Fri, Mar 04, 2016 at 07:03:09PM +0100, Marek Polacek wrote:
> On Fri, Mar 04, 2016 at 06:41:26PM +0100, Jakub Jelinek wrote:
> > I'm ok with it for gcc6.
>
> Cool.
>
> > But IMHO you should add dg-bogus directives here.
>
> Ok, version with dg-bogus:
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-03-04 Marek Polacek <polacek@redhat.com>
>
> PR c/69407
> * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
> operations.
>
> * gcc.dg/atomic-op-6.c: New test.
>
> diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
> index 965cf49..25afa9c 100644
> --- gcc/c-family/c-common.c
> +++ gcc/c-family/c-common.c
> @@ -11443,6 +11443,10 @@ resolve_overloaded_builtin (location_t loc, tree function,
> && orig_code != BUILT_IN_ATOMIC_STORE_N)
> result = sync_resolve_return (first_param, result, orig_format);
>
> + if (fetch_op)
> + /* Prevent -Wunused-value warning. */
> + TREE_USED (result) = true;
> +
> /* If new_return is set, assign function to that expr and cast the
> result to void since the generic interface returned void. */
> if (new_return)
> diff --git gcc/testsuite/gcc.dg/atomic-op-6.c gcc/testsuite/gcc.dg/atomic-op-6.c
> index e69de29..f88c293 100644
> --- gcc/testsuite/gcc.dg/atomic-op-6.c
> +++ gcc/testsuite/gcc.dg/atomic-op-6.c
> @@ -0,0 +1,11 @@
> +/* Test we don't generate bogus warnings. */
> +/* PR c/69407 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall -Wextra" } */
> +
> +void
> +foo (int *p, int a)
> +{
> + __atomic_fetch_add (&p, a, 0); /* { dg-bogus "value computed is not used" } */
> + __atomic_add_fetch (&p, a, 0); /* { dg-bogus "value computed is not used" } */
> +}
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-14 11:48 ` Marek Polacek
@ 2016-03-17 17:24 ` Jeff Law
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2016-03-17 17:24 UTC (permalink / raw)
To: Marek Polacek, Jakub Jelinek; +Cc: GCC Patches, Joseph Myers
On 03/14/2016 05:48 AM, Marek Polacek wrote:
> Ping.
>
> On Fri, Mar 04, 2016 at 07:03:09PM +0100, Marek Polacek wrote:
>> On Fri, Mar 04, 2016 at 06:41:26PM +0100, Jakub Jelinek wrote:
>>> I'm ok with it for gcc6.
>>
>> Cool.
>>
>>> But IMHO you should add dg-bogus directives here.
>>
>> Ok, version with dg-bogus:
>>
>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>
>> 2016-03-04 Marek Polacek <polacek@redhat.com>
>>
>> PR c/69407
>> * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
>> operations.
>>
>> * gcc.dg/atomic-op-6.c: New test.
OK.
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-18 15:46 ` Uros Bizjak
@ 2016-03-18 16:02 ` Uros Bizjak
0 siblings, 0 replies; 11+ messages in thread
From: Uros Bizjak @ 2016-03-18 16:02 UTC (permalink / raw)
To: Marek Polacek
Cc: gcc-patches, Joseph S. Myers, Jakub Jelinek, Jonathan Wakely
On Fri, Mar 18, 2016 at 4:33 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Mar 7, 2016 at 2:34 PM, Marek Polacek <polacek@redhat.com> wrote:
>> On Fri, Mar 04, 2016 at 07:17:46PM +0100, Uros Bizjak wrote:
>>> Hello!
>>>
>>> > This is not a regression but I thought I'd post this anyway. Martin reported
>>> > that we generate -Wunused-value warnings on the attached testcase, which
>>> > arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since
>>> > we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g.
>>> > for __atomic_load/store/compare.)
>>> >
>>> > Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7?
>>> >
>>> > 2016-03-04 Marek Polacek <polacek@redhat.com>
>>> >
>>> > PR c/69407
>>> > * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
>>> > operations.
>>> >
>>> > * gcc.dg/atomic-op-6.c: New test.
>>>
>>> You can probably revert my workaround [1] that suppressed these
>>> warnings in libsupc++/guard.cc.
>>
>> Ah, thanks for the heads-up, I'll do that once I get the patch in.
>
> I have committed the attached revert after bootstrap on
> x86_64-linux-gnu {,-m32}. There were no warnings when compiling
> guard.cc.
>
> 2016-03-18 Uros Bizjak <ubizjak@gmail.com>
>
> Revert:
> 2015-07-02 Uros Bizjak <ubizjak@gmail.com>
>
> * libsupc++/guard.cc (__test_and_acquire): Use __p after __atomic_load
> to avoid unused variable warning.
> (__set_and_release): Use __p after __atomic_store to avoid unused
> variable warning.
Whoops, I looked at the wrong part of the build log ... unfortunately,
the warning still happens, and I have to revert the revert ...
Sorry for the noise,
Uros.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-07 13:34 ` Marek Polacek
@ 2016-03-18 15:46 ` Uros Bizjak
2016-03-18 16:02 ` Uros Bizjak
0 siblings, 1 reply; 11+ messages in thread
From: Uros Bizjak @ 2016-03-18 15:46 UTC (permalink / raw)
To: Marek Polacek
Cc: gcc-patches, Joseph S. Myers, Jakub Jelinek, Jonathan Wakely
[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]
On Mon, Mar 7, 2016 at 2:34 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Fri, Mar 04, 2016 at 07:17:46PM +0100, Uros Bizjak wrote:
>> Hello!
>>
>> > This is not a regression but I thought I'd post this anyway. Martin reported
>> > that we generate -Wunused-value warnings on the attached testcase, which
>> > arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since
>> > we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g.
>> > for __atomic_load/store/compare.)
>> >
>> > Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7?
>> >
>> > 2016-03-04 Marek Polacek <polacek@redhat.com>
>> >
>> > PR c/69407
>> > * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
>> > operations.
>> >
>> > * gcc.dg/atomic-op-6.c: New test.
>>
>> You can probably revert my workaround [1] that suppressed these
>> warnings in libsupc++/guard.cc.
>
> Ah, thanks for the heads-up, I'll do that once I get the patch in.
I have committed the attached revert after bootstrap on
x86_64-linux-gnu {,-m32}. There were no warnings when compiling
guard.cc.
2016-03-18 Uros Bizjak <ubizjak@gmail.com>
Revert:
2015-07-02 Uros Bizjak <ubizjak@gmail.com>
* libsupc++/guard.cc (__test_and_acquire): Use __p after __atomic_load
to avoid unused variable warning.
(__set_and_release): Use __p after __atomic_store to avoid unused
variable warning.
Uros.
[-- Attachment #2: l.diff.txt --]
[-- Type: text/plain, Size: 788 bytes --]
Index: libsupc++/guard.cc
===================================================================
--- libsupc++/guard.cc (revision 234330)
+++ libsupc++/guard.cc (working copy)
@@ -117,7 +117,6 @@ __test_and_acquire (__cxxabiv1::__guard *g)
unsigned char __c;
unsigned char *__p = reinterpret_cast<unsigned char *>(g);
__atomic_load (__p, &__c, __ATOMIC_ACQUIRE);
- (void) __p;
return _GLIBCXX_GUARD_TEST(&__c);
}
# define _GLIBCXX_GUARD_TEST_AND_ACQUIRE(G) __test_and_acquire (G)
@@ -133,7 +132,6 @@ __set_and_release (__cxxabiv1::__guard *g)
unsigned char *__p = reinterpret_cast<unsigned char *>(g);
unsigned char val = 1;
__atomic_store (__p, &val, __ATOMIC_RELEASE);
- (void) __p;
}
# define _GLIBCXX_GUARD_SET_AND_RELEASE(G) __set_and_release (G)
# endif
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
2016-03-04 18:17 Uros Bizjak
@ 2016-03-07 13:34 ` Marek Polacek
2016-03-18 15:46 ` Uros Bizjak
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2016-03-07 13:34 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches, Joseph S. Myers, Jakub Jelinek, Jonathan Wakely
On Fri, Mar 04, 2016 at 07:17:46PM +0100, Uros Bizjak wrote:
> Hello!
>
> > This is not a regression but I thought I'd post this anyway. Martin reported
> > that we generate -Wunused-value warnings on the attached testcase, which
> > arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since
> > we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g.
> > for __atomic_load/store/compare.)
> >
> > Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7?
> >
> > 2016-03-04 Marek Polacek <polacek@redhat.com>
> >
> > PR c/69407
> > * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
> > operations.
> >
> > * gcc.dg/atomic-op-6.c: New test.
>
> You can probably revert my workaround [1] that suppressed these
> warnings in libsupc++/guard.cc.
Ah, thanks for the heads-up, I'll do that once I get the patch in.
> [1] https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00023.html
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
@ 2016-03-04 18:17 Uros Bizjak
2016-03-07 13:34 ` Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Uros Bizjak @ 2016-03-04 18:17 UTC (permalink / raw)
To: gcc-patches
Cc: Marek Polacek, Joseph S. Myers, Jakub Jelinek, Jonathan Wakely
Hello!
> This is not a regression but I thought I'd post this anyway. Martin reported
> that we generate -Wunused-value warnings on the attached testcase, which
> arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since
> we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g.
> for __atomic_load/store/compare.)
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7?
>
> 2016-03-04 Marek Polacek <polacek@redhat.com>
>
> PR c/69407
> * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
> operations.
>
> * gcc.dg/atomic-op-6.c: New test.
You can probably revert my workaround [1] that suppressed these
warnings in libsupc++/guard.cc.
[1] https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00023.html
Uros.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-03-18 15:46 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-04 17:30 [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407) Marek Polacek
2016-03-04 17:41 ` Jakub Jelinek
2016-03-04 18:03 ` Marek Polacek
2016-03-04 18:20 ` Jakub Jelinek
2016-03-07 13:33 ` Marek Polacek
2016-03-14 11:48 ` Marek Polacek
2016-03-17 17:24 ` Jeff Law
2016-03-04 18:17 Uros Bizjak
2016-03-07 13:34 ` Marek Polacek
2016-03-18 15:46 ` Uros Bizjak
2016-03-18 16:02 ` Uros Bizjak
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).