* [PATCH roland/arm-atomic-warn] Fiddle ARM atomic.h to avoid -Wvolatile-register-var warnings.
@ 2012-08-08 20:44 Roland McGrath
2012-08-08 20:52 ` Joseph S. Myers
2012-08-08 22:02 ` Richard Henderson
0 siblings, 2 replies; 4+ messages in thread
From: Roland McGrath @ 2012-08-08 20:44 UTC (permalink / raw)
To: libc-ports
Trunk GCC has -Wvolatile-register-var on by default and this hits in
some atomic.h macros because __typeof (*ptr) is leaking volatile-ness
into the non-pointer declarations and being treated as 'register volatile'.
I only did a basic compile test (with trunk gcc for arm-linux-gnueabi).
I looked at one file where I'd been getting the warning (nptl-init.c)
and the generated code differed only in reordering a couple of (non-atomic)
loads whose order shouldn't matter.
Ok?
Thanks,
Roland
ports/ChangeLog.arm
* sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h
[!__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4]
(__arch_compare_and_exchange_val_32_acq): Use uint32_t rather than
__typeof (...) for non-pointer variables derived from the arguments.
diff --git a/ports/sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h b/ports/sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h
index 4e810a2..c9ad50d 100644
--- a/ports/sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h
+++ b/ports/sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
+/* Copyright (C) 2002-2012 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -74,17 +74,24 @@ void __arm_link_error (void);
/* It doesn't matter what register is used for a_oldval2, but we must
specify one to work around GCC PR rtl-optimization/21223. Otherwise
- it may cause a_oldval or a_tmp to be moved to a different register. */
+ it may cause a_oldval or a_tmp to be moved to a different register.
+ We use the union trick rather than simply using __typeof (...) in the
+ declarations of A_OLDVAL et al because when NEWVAL or OLDVAL is of the
+ form *PTR and PTR has a 'volatile ... *' type, then __typeof (*PTR) has
+ a 'volatile ...' type and this triggers -Wvolatile-register-var to
+ complain about 'register volatile ... asm ("reg")'. */
#elif defined __thumb2__
/* Thumb-2 has ldrex/strex. However it does not have barrier instructions,
so we still need to use the kernel helper. */
#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \
- ({ register __typeof (oldval) a_oldval asm ("r0"); \
- register __typeof (oldval) a_newval asm ("r1") = (newval); \
+ ({ union { __typeof (oldval) a; uint32_t v; } oldval_arg = { .a = (oldval) };\
+ union { __typeof (newval) a; uint32_t v; } newval_arg = { .a = (newval) };\
+ register uint32_t a_oldval asm ("r0"); \
+ register uint32_t a_newval asm ("r1") = newval_arg.v; \
register __typeof (mem) a_ptr asm ("r2") = (mem); \
- register __typeof (oldval) a_tmp asm ("r3"); \
- register __typeof (oldval) a_oldval2 asm ("r4") = (oldval); \
+ register uint32_t a_tmp asm ("r3"); \
+ register uint32_t a_oldval2 asm ("r4") = oldval_arg.v; \
__asm__ __volatile__ \
("0:\tldr\t%[tmp],[%[ptr]]\n\t" \
"cmp\t%[tmp], %[old2]\n\t" \
@@ -100,14 +107,16 @@ void __arm_link_error (void);
: [new] "r" (a_newval), [ptr] "r" (a_ptr), \
[old2] "r" (a_oldval2) \
: "ip", "lr", "cc", "memory"); \
- a_tmp; })
+ (__typeof (oldval)) a_tmp; })
#else
-#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \
- ({ register __typeof (oldval) a_oldval asm ("r0"); \
- register __typeof (oldval) a_newval asm ("r1") = (newval); \
+#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \
+ ({ union { __typeof (oldval) a; uint32_t v; } oldval_arg = { .a = (oldval) };\
+ union { __typeof (newval) a; uint32_t v; } newval_arg = { .a = (newval) };\
+ register uint32_t a_oldval asm ("r0"); \
+ register uint32_t a_newval asm ("r1") = newval_arg.v; \
register __typeof (mem) a_ptr asm ("r2") = (mem); \
- register __typeof (oldval) a_tmp asm ("r3"); \
- register __typeof (oldval) a_oldval2 asm ("r4") = (oldval); \
+ register uint32_t a_tmp asm ("r3"); \
+ register uint32_t a_oldval2 asm ("r4") = oldval_arg.v; \
__asm__ __volatile__ \
("0:\tldr\t%[tmp],[%[ptr]]\n\t" \
"cmp\t%[tmp], %[old2]\n\t" \
@@ -123,7 +132,7 @@ void __arm_link_error (void);
: [new] "r" (a_newval), [ptr] "r" (a_ptr), \
[old2] "r" (a_oldval2) \
: "ip", "lr", "cc", "memory"); \
- a_tmp; })
+ (__typeof (oldval)) a_tmp; })
#endif
#define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH roland/arm-atomic-warn] Fiddle ARM atomic.h to avoid -Wvolatile-register-var warnings.
2012-08-08 20:44 [PATCH roland/arm-atomic-warn] Fiddle ARM atomic.h to avoid -Wvolatile-register-var warnings Roland McGrath
@ 2012-08-08 20:52 ` Joseph S. Myers
2012-08-08 22:02 ` Richard Henderson
1 sibling, 0 replies; 4+ messages in thread
From: Joseph S. Myers @ 2012-08-08 20:52 UTC (permalink / raw)
To: Roland McGrath; +Cc: libc-ports
On Wed, 8 Aug 2012, Roland McGrath wrote:
> Trunk GCC has -Wvolatile-register-var on by default and this hits in
> some atomic.h macros because __typeof (*ptr) is leaking volatile-ness
> into the non-pointer declarations and being treated as 'register volatile'.
>
> I only did a basic compile test (with trunk gcc for arm-linux-gnueabi).
> I looked at one file where I'd been getting the warning (nptl-init.c)
> and the generated code differed only in reordering a couple of (non-atomic)
> loads whose order shouldn't matter.
>
> Ok?
OK.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH roland/arm-atomic-warn] Fiddle ARM atomic.h to avoid -Wvolatile-register-var warnings.
2012-08-08 20:44 [PATCH roland/arm-atomic-warn] Fiddle ARM atomic.h to avoid -Wvolatile-register-var warnings Roland McGrath
2012-08-08 20:52 ` Joseph S. Myers
@ 2012-08-08 22:02 ` Richard Henderson
2012-08-08 22:13 ` Roland McGrath
1 sibling, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2012-08-08 22:02 UTC (permalink / raw)
To: Roland McGrath; +Cc: libc-ports
On 08/08/2012 01:44 PM, Roland McGrath wrote:
> + We use the union trick rather than simply using __typeof (...) in the
> + declarations of A_OLDVAL et al because when NEWVAL or OLDVAL is of the
> + form *PTR and PTR has a 'volatile ... *' type, then __typeof (*PTR) has
> + a 'volatile ...' type and this triggers -Wvolatile-register-var to
> + complain about 'register volatile ... asm ("reg")'. */
> #elif defined __thumb2__
> /* Thumb-2 has ldrex/strex. However it does not have barrier instructions,
> so we still need to use the kernel helper. */
> #define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \
> - ({ register __typeof (oldval) a_oldval asm ("r0"); \
> - register __typeof (oldval) a_newval asm ("r1") = (newval); \
> + ({ union { __typeof (oldval) a; uint32_t v; } oldval_arg = { .a = (oldval) };\
> + union { __typeof (newval) a; uint32_t v; } newval_arg = { .a = (newval) };\
> + register uint32_t a_oldval asm ("r0"); \
> + register uint32_t a_newval asm ("r1") = newval_arg.v; \
> register __typeof (mem) a_ptr asm ("r2") = (mem); \
> - register __typeof (oldval) a_tmp asm ("r3"); \
> - register __typeof (oldval) a_oldval2 asm ("r4") = (oldval); \
> + register uint32_t a_tmp asm ("r3"); \
> + register uint32_t a_oldval2 asm ("r4") = oldval_arg.v; \
> __asm__ __volatile__ \
> ("0:\tldr\t%[tmp],[%[ptr]]\n\t" \
Is it really worth working so hard to avoid the gcc builtins?
Yes, gcc before 4.7 doesn't have __atomic to avoid the extra
barriers, but at least 4.7 and later will.
May I ask what gcc version you're targeting, anyway?
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH roland/arm-atomic-warn] Fiddle ARM atomic.h to avoid -Wvolatile-register-var warnings.
2012-08-08 22:02 ` Richard Henderson
@ 2012-08-08 22:13 ` Roland McGrath
0 siblings, 0 replies; 4+ messages in thread
From: Roland McGrath @ 2012-08-08 22:13 UTC (permalink / raw)
To: Richard Henderson; +Cc: libc-ports
> Is it really worth working so hard to avoid the gcc builtins?
> Yes, gcc before 4.7 doesn't have __atomic to avoid the extra
> barriers, but at least 4.7 and later will.
That file uses them #ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4.
I was just killing a warning without worrying about why the code
path was being used.
> May I ask what gcc version you're targeting, anyway?
I'm using today's trunk, cross --target=arm-linux-gnueabi and no other
special options. I don't know why __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 is
not defined. Probably it would be with appropriate -march= settings.
Thanks,
Roland
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-08 22:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-08 20:44 [PATCH roland/arm-atomic-warn] Fiddle ARM atomic.h to avoid -Wvolatile-register-var warnings Roland McGrath
2012-08-08 20:52 ` Joseph S. Myers
2012-08-08 22:02 ` Richard Henderson
2012-08-08 22:13 ` Roland McGrath
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).