[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: i386 byteswap.h provoking ``may be undefined'' warnings in gcc3
From: |
Paul Haahr |
Subject: |
Re: i386 byteswap.h provoking ``may be undefined'' warnings in gcc3 |
Date: |
Thu, 12 Jul 2001 02:48:29 -0700 (PDT) |
Andreas wrote, replying to me:
> > Gcc 3.0's new ``warning: operation on `p' may be undefined'' is
> > (spuriously?) provoked by glibc's <bits/byteswap.h> code for the x86.
> >
> > [...]
> >
> > The bswap_16 and bswap_32 macros expand into horrible creatures, where a
> > reference to n++ appears several times in one expression. This generates
> > one warning for the call to bswap_16 and three for the call to bswap_32.
> > However, evaluation of the problematic expressions is gated by an
> > ``if (__builtin_constant_p(x))'' test which always evaluates to false if
> > there are internal side effects, so the ``may be undefined'' path is
> > never actually taken.
> >
> > I can't quite tell if this is a gcc bug or a glibc bug, but I've
> > attached a workaround that lives in glibc. It appears to still
> > constant fold properly, and generate identical code to the old
> > version, for a few small test cases.
>
> But might fail with GCC 3.1 if that one has a more sophisticated
> analysis.
I don't think so, because my workaround uses an additional local
variable to avoid repeated evaluation of the macro argument, so the
internal multiple evaluations in the macro are not a problem.
I've reattached my proposed patch -- take a look at the new variable __x
for why the problem doesn't exist in my version.
> > Fixing this in gcc would require not issuing this kind of warning
> > (and perhaps others) in code that isn't evaluated due to a
> > __builtin_constant_p test.
>
> And that's the way to go.
That's still not clear -- it's hard to argue that, just because some
code is in a provably untaken branch of an if, it shouldn't be checked
for warnings. And special casing __builtin_constant_p seems problematic.
Any gcc folk want to weigh in with an opinion?
--p
--- /usr/include/bits/byteswap.h Sun Nov 19 06:24:03 2000
+++ bits/byteswap.h Wed Jul 11 15:22:48 2001
@@ -29,9 +29,10 @@
# define __bswap_16(x) \
(__extension__ \
({ register unsigned short int __v; \
- if (__builtin_constant_p (x)) \
- __v = __bswap_constant_16 (x); \
- else \
+ if (__builtin_constant_p (x)) { \
+ register unsigned short int __x = x; \
+ __v = __bswap_constant_16 (__x); \
+ } else \
__asm__ __volatile__ ("rorw $8, %w0" \
: "=r" (__v) \
: "0" ((unsigned short int) (x)) \
@@ -55,9 +56,10 @@
# define __bswap_32(x) \
(__extension__ \
({ register unsigned int __v; \
- if (__builtin_constant_p (x)) \
- __v = __bswap_constant_32 (x); \
- else \
+ if (__builtin_constant_p (x)) { \
+ register unsigned int __x = x; \
+ __v = __bswap_constant_32 (__x); \
+ } else \
__asm__ __volatile__ ("rorw $8, %w0;" \
"rorl $16, %0;" \
"rorw $8, %w0" \