* [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast
@ 2016-08-07 1:41 Pranith Kumar
2016-08-08 9:05 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Pranith Kumar @ 2016-08-07 1:41 UTC (permalink / raw)
To: Richard Henderson, Emilio G. Cota, Alex Bennée,
Sergey Fedorov, Markus Armbruster, open list:All patches CC here
With the latest clang, we have the following warning. We are not using
the const qualifier consistently in other functions. So remove it from
the only one that uses it to fix the warning.
/home/pranith/devops/code/qemu/include/qemu/seqlock.h:62:21: warning: passing 'typeof (*&sl->sequence) *' (aka 'const unsigned int *') to parameter of type 'unsigned int *' discards qualifier
s [-Wincompatible-pointer-types-discards-qualifiers]
return unlikely(atomic_read(&sl->sequence) != start);
^~~~~~~~~~~~~~~~~~~~~~~~~~
/home/pranith/devops/code/qemu/include/qemu/atomic.h:58:25: note: expanded from macro 'atomic_read'
__atomic_load(ptr, &_val, __ATOMIC_RELAXED); \
^~~~~
/home/pranith/devops/code/qemu/include/qemu/compiler.h:62:43: note: expanded from macro 'unlikely'
#define unlikely(x) __builtin_expect(!!(x), 0)
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
include/qemu/seqlock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h
index 2e2be4c..aa4cf15 100644
--- a/include/qemu/seqlock.h
+++ b/include/qemu/seqlock.h
@@ -55,7 +55,7 @@ static inline unsigned seqlock_read_begin(QemuSeqLock *sl)
return ret & ~1;
}
-static inline int seqlock_read_retry(const QemuSeqLock *sl, unsigned start)
+static inline int seqlock_read_retry(QemuSeqLock *sl, unsigned start)
{
/* Read other fields before reading final sequence. */
smp_rmb();
--
2.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast 2016-08-07 1:41 [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast Pranith Kumar @ 2016-08-08 9:05 ` Paolo Bonzini 2016-08-08 9:27 ` Paolo Bonzini 0 siblings, 1 reply; 7+ messages in thread From: Paolo Bonzini @ 2016-08-08 9:05 UTC (permalink / raw) To: Pranith Kumar, Richard Henderson, Emilio G. Cota, Alex Bennée, Sergey Fedorov, Markus Armbruster, open list:All patches CC here On 07/08/2016 03:41, Pranith Kumar wrote: > With the latest clang, we have the following warning. We are not using > the const qualifier consistently in other functions. So remove it from > the only one that uses it to fix the warning. > > /home/pranith/devops/code/qemu/include/qemu/seqlock.h:62:21: warning: passing 'typeof (*&sl->sequence) *' (aka 'const unsigned int *') to parameter of type 'unsigned int *' discards qualifier > s [-Wincompatible-pointer-types-discards-qualifiers] > return unlikely(atomic_read(&sl->sequence) != start); > ^~~~~~~~~~~~~~~~~~~~~~~~~~ > /home/pranith/devops/code/qemu/include/qemu/atomic.h:58:25: note: expanded from macro 'atomic_read' > __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ > ^~~~~ > /home/pranith/devops/code/qemu/include/qemu/compiler.h:62:43: note: expanded from macro 'unlikely' > #define unlikely(x) __builtin_expect(!!(x), 0) > > Signed-off-by: Pranith Kumar <bobby.prani@gmail.com> This is a compiler bug, isn't it? Atomic loads of a const pointer should be allowed. Paolo > --- > include/qemu/seqlock.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h > index 2e2be4c..aa4cf15 100644 > --- a/include/qemu/seqlock.h > +++ b/include/qemu/seqlock.h > @@ -55,7 +55,7 @@ static inline unsigned seqlock_read_begin(QemuSeqLock *sl) > return ret & ~1; > } > > -static inline int seqlock_read_retry(const QemuSeqLock *sl, unsigned start) > +static inline int seqlock_read_retry(QemuSeqLock *sl, unsigned start) > { > /* Read other fields before reading final sequence. */ > smp_rmb(); > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast 2016-08-08 9:05 ` Paolo Bonzini @ 2016-08-08 9:27 ` Paolo Bonzini 2016-08-08 9:55 ` Paolo Bonzini 0 siblings, 1 reply; 7+ messages in thread From: Paolo Bonzini @ 2016-08-08 9:27 UTC (permalink / raw) To: Pranith Kumar, Richard Henderson, Emilio G. Cota, Alex Bennée, Sergey Fedorov, Markus Armbruster, open list:All patches CC here On 08/08/2016 11:05, Paolo Bonzini wrote: >> > /home/pranith/devops/code/qemu/include/qemu/seqlock.h:62:21: warning: passing 'typeof (*&sl->sequence) *' (aka 'const unsigned int *') to parameter of type 'unsigned int *' discards qualifier >> > s [-Wincompatible-pointer-types-discards-qualifiers] >> > return unlikely(atomic_read(&sl->sequence) != start); >> > ^~~~~~~~~~~~~~~~~~~~~~~~~~ >> > /home/pranith/devops/code/qemu/include/qemu/atomic.h:58:25: note: expanded from macro 'atomic_read' >> > __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ >> > ^~~~~ >> > /home/pranith/devops/code/qemu/include/qemu/compiler.h:62:43: note: expanded from macro 'unlikely' >> > #define unlikely(x) __builtin_expect(!!(x), 0) >> > >> > Signed-off-by: Pranith Kumar <bobby.prani@gmail.com> > This is a compiler bug, isn't it? Atomic loads of a const pointer > should be allowed. Oh, this is &_val having a const type. That makes more sense indeed, but it should be worked around in qemu/atomic.h. Can you check if this works? diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h index 7e13fca..9c664a7 100644 --- a/include/qemu/atomic.h +++ b/include/qemu/atomic.h @@ -54,7 +54,7 @@ #define atomic_read(ptr) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _val; \ + typeof(*ptr+0) _val; \ __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ _val; \ }) @@ -80,7 +80,7 @@ #define atomic_rcu_read(ptr) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _val; \ + typeof(*ptr+0) _val; \ atomic_rcu_read__nocheck(ptr, &_val); \ _val; \ }) @@ -103,7 +103,7 @@ #define atomic_mb_read(ptr) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _val; \ + typeof(*ptr+0) _val; \ __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ smp_rmb(); \ _val; \ @@ -120,7 +120,7 @@ #define atomic_mb_read(ptr) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _val; \ + typeof(*ptr+0) _val; \ __atomic_load(ptr, &_val, __ATOMIC_SEQ_CST); \ _val; \ }) @@ -137,7 +137,7 @@ #define atomic_xchg(ptr, i) ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _new = (i), _old; \ + typeof(*ptr+0) _new = (i), _old; \ __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \ _old; \ }) @@ -146,7 +146,7 @@ #define atomic_cmpxchg(ptr, old, new) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _old = (old), _new = (new); \ + typeof(*ptr+0) _old = (old), _new = (new); \ __atomic_compare_exchange(ptr, &_old, &_new, false, \ __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); \ _old; \ ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast 2016-08-08 9:27 ` Paolo Bonzini @ 2016-08-08 9:55 ` Paolo Bonzini 2016-08-08 10:00 ` Richard Henderson 0 siblings, 1 reply; 7+ messages in thread From: Paolo Bonzini @ 2016-08-08 9:55 UTC (permalink / raw) To: Pranith Kumar, Richard Henderson, Emilio G. Cota, Alex Bennée, Sergey Fedorov, Markus Armbruster, open list:All patches CC here On 08/08/2016 11:27, Paolo Bonzini wrote: > > > On 08/08/2016 11:05, Paolo Bonzini wrote: >>>> /home/pranith/devops/code/qemu/include/qemu/seqlock.h:62:21: warning: passing 'typeof (*&sl->sequence) *' (aka 'const unsigned int *') to parameter of type 'unsigned int *' discards qualifier >>>> s [-Wincompatible-pointer-types-discards-qualifiers] >>>> return unlikely(atomic_read(&sl->sequence) != start); >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> /home/pranith/devops/code/qemu/include/qemu/atomic.h:58:25: note: expanded from macro 'atomic_read' >>>> __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ >>>> ^~~~~ >>>> /home/pranith/devops/code/qemu/include/qemu/compiler.h:62:43: note: expanded from macro 'unlikely' >>>> #define unlikely(x) __builtin_expect(!!(x), 0) >>>> >>>> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com> >> This is a compiler bug, isn't it? Atomic loads of a const pointer >> should be allowed. > > Oh, this is &_val having a const type. That makes more sense indeed, > but it should be worked around in qemu/atomic.h. > > Can you check if this works? Better: diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h index 7e13fca..81cd33d 100644 --- a/include/qemu/atomic.h +++ b/include/qemu/atomic.h @@ -18,6 +18,26 @@ /* Compiler barrier */ #define barrier() ({ asm volatile("" ::: "memory"); (void)0; }) +#define typeof_strip_const(expr) \ + typeof( \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), const short) || \ + __builtin_types_compatible_p(typeof(expr), short), \ + (short)1, \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), const unsigned short) || \ + __builtin_types_compatible_p(typeof(expr), unsigned short), \ + (unsigned short)1, \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), const char) || \ + __builtin_types_compatible_p(typeof(expr), char), \ + (char)1, \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), const unsigned char) || \ + __builtin_types_compatible_p(typeof(expr), unsigned char), \ + (unsigned char)1, \ + expr+0))))) + #ifdef __ATOMIC_RELAXED /* For C11 atomic ops */ @@ -54,7 +74,7 @@ #define atomic_read(ptr) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _val; \ + typeof_strip_const(*ptr) _val; \ __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ _val; \ }) @@ -80,7 +100,7 @@ #define atomic_rcu_read(ptr) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _val; \ + typeof_strip_const(*ptr) _val; \ atomic_rcu_read__nocheck(ptr, &_val); \ _val; \ }) @@ -103,7 +123,7 @@ #define atomic_mb_read(ptr) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _val; \ + typeof_strip_const(*ptr) _val; \ __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ smp_rmb(); \ _val; \ @@ -120,7 +140,7 @@ #define atomic_mb_read(ptr) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _val; \ + typeof_strip_const(*ptr) _val; \ __atomic_load(ptr, &_val, __ATOMIC_SEQ_CST); \ _val; \ }) @@ -137,7 +157,7 @@ #define atomic_xchg(ptr, i) ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _new = (i), _old; \ + typeof_strip_const(*ptr) _new = (i), _old; \ __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \ _old; \ }) @@ -146,7 +166,7 @@ #define atomic_cmpxchg(ptr, old, new) \ ({ \ QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - typeof(*ptr) _old = (old), _new = (new); \ + typeof_strip_const(*ptr) _old = (old), _new = (new); \ __atomic_compare_exchange(ptr, &_old, &_new, false, \ __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); \ _old; \ Paolo ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast 2016-08-08 9:55 ` Paolo Bonzini @ 2016-08-08 10:00 ` Richard Henderson 2016-08-08 10:02 ` Paolo Bonzini 0 siblings, 1 reply; 7+ messages in thread From: Richard Henderson @ 2016-08-08 10:00 UTC (permalink / raw) To: Paolo Bonzini, Pranith Kumar, Emilio G. Cota, Alex Bennée, Sergey Fedorov, Markus Armbruster, open list:All patches CC here On 08/08/2016 03:25 PM, Paolo Bonzini wrote: > + __builtin_choose_expr( \ > + __builtin_types_compatible_p(typeof(expr), const char) || \ > + __builtin_types_compatible_p(typeof(expr), char), \ > + (char)1, \ Better as signed char, since bare char could be either signed or unsigned. r~ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast 2016-08-08 10:00 ` Richard Henderson @ 2016-08-08 10:02 ` Paolo Bonzini 2016-08-08 10:25 ` Richard Henderson 0 siblings, 1 reply; 7+ messages in thread From: Paolo Bonzini @ 2016-08-08 10:02 UTC (permalink / raw) To: Richard Henderson, Pranith Kumar, Emilio G. Cota, Alex Bennée, Sergey Fedorov, Markus Armbruster, open list:All patches CC here On 08/08/2016 12:00, Richard Henderson wrote: > On 08/08/2016 03:25 PM, Paolo Bonzini wrote: >> + >> __builtin_choose_expr( \ >> + __builtin_types_compatible_p(typeof(expr), const char) >> || \ >> + __builtin_types_compatible_p(typeof(expr), >> char), \ >> + >> (char)1, \ > > Better as signed char, since bare char could be either signed or unsigned. It's also better to add a volatile case. Paolo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast 2016-08-08 10:02 ` Paolo Bonzini @ 2016-08-08 10:25 ` Richard Henderson 0 siblings, 0 replies; 7+ messages in thread From: Richard Henderson @ 2016-08-08 10:25 UTC (permalink / raw) To: Paolo Bonzini, Pranith Kumar, Emilio G. Cota, Alex Bennée, Sergey Fedorov, Markus Armbruster, open list:All patches CC here On 08/08/2016 03:32 PM, Paolo Bonzini wrote: > > > On 08/08/2016 12:00, Richard Henderson wrote: >> On 08/08/2016 03:25 PM, Paolo Bonzini wrote: >>> + >>> __builtin_choose_expr( \ >>> + __builtin_types_compatible_p(typeof(expr), const char) >>> || \ >>> + __builtin_types_compatible_p(typeof(expr), >>> char), \ >>> + >>> (char)1, \ >> >> Better as signed char, since bare char could be either signed or unsigned. > > It's also better to add a volatile case. ... and, eventually, the _Atomic case. I really should have finished the __typeof_noqual patch that I was working on some years ago... Ho hum. r~ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-08-08 10:25 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-08-07 1:41 [Qemu-devel] [PATCH 1/1] seqlock: Fix warning reg. incompatible cast Pranith Kumar 2016-08-08 9:05 ` Paolo Bonzini 2016-08-08 9:27 ` Paolo Bonzini 2016-08-08 9:55 ` Paolo Bonzini 2016-08-08 10:00 ` Richard Henderson 2016-08-08 10:02 ` Paolo Bonzini 2016-08-08 10:25 ` Richard Henderson
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 NNTP newsgroup(s).