Changes between Version 16 and Version 17 of Guidelines/WarningsGuidelines

Timestamp:

Jan 20, 2011, 12:49:09 AM (12 years ago)

Author:

phorgan1

Comment:

--

Guidelines/WarningsGuidelines

@@ -359 +359 @@
 '''-pedantic'''
 
-    This instructs GCC to consider the release version of C or C++ and to first issue all warnings required by that version of the standard, and second warn about the use of gcc extenstions to the language.  This would, for instance warn about the use of ''#ident'' since it is a gcc extension and is not in any of the standards.  Using -pedantic makes it easier to write portable code, or at least to know when it is not.  Best used with the ''-std=xxxxx'' argument to the compiler, so that you know what version of the standard you're being compared to.  Currently if ''-std=xxxxx'' is not specified, it's as if you'd written ''-std=gnu++98''.  Much of boost uses facilities from more recent versions of the standard, so it might make more sense to explicitly specify ''-std=c++0x'' or ''-std=gnu++0x''.  ''-std=c++0x'' is most portable.  See the page [http://gcc.gnu.org/onlinedocs/gcc/Standards.html] for more information.
+    This instructs GCC to consider the release version of C or C++ and to first issue all warnings required by that version of the standard, and second warn about the use of gcc extensions to the language.  This would, for instance warn about the use of ''#ident'' since it is a gcc extension and is not in any of the standards.  Using -pedantic makes it easier to write portable code, or at least to know when it is not.  Best used with the ''-std=xxxxx'' argument to the compiler, so that you know what version of the standard you're being compared to.  Currently if ''-std=xxxxx'' is not specified, it's as if you'd written ''-std=gnu++98''.  Much of boost uses facilities from more recent versions of the standard, so it might make more sense to explicitly specify ''-std=c++0x'' or ''-std=gnu++0x''.  ''-std=c++0x'' is most portable.  See the page [http://gcc.gnu.org/onlinedocs/gcc/Standards.html] for more information.
 
 
@@ -564 +564 @@
 
 THAT's what the warning is trying to tell you, that the optimizer is going to do things that you don't like.  In this case seeing that acopy is set to a and never touched again, strict aliasing lets it optimize by just returning the original value of a at the end.
+==== Broken version ====
 {{{
 uint32_t
@@ -586 +587 @@
 }
 }}}
+So what goes wrong? Since a uint16_t can't alias a uint32_t, under the rules, it's ignored in
+considering what to do with acopy. Since it sees that nothing is done with acopy inside the
+swaphalves function, it just returns the original value of a.
+
 Here's the (annotated) x86 assembler generated by gcc 4.4.1 for swaphalves, let's see what went wrong:
 {{{
@@ -612 +617 @@
     ret
 }}}
-Scary, isn't it?  Of course you could use -fno-strict-aliasing to get the right output, but the generated code won't be as good.  A better way to accomplish the same thing without the warning's or the incorrect output is to define swaphalves like this:
+Scary, isn't it? Of course, if you are using gcc, you could use -fno-strict-aliasing to get the right output,
+but the generated code won't be as good. A better way to accomplish the same thing without the
+warnings or the incorrect output is to define swaphalves like this. N.B. this is supported in C99 and
+later C specs, as noted in this footnote to 6.5.2.3 Structure and union members:[[BR]]
+{{{
+85)
+If the member used to access the contents of a union object is not the 
+same as the member last used to store a value in the object, the appropriate 
+part of the object representation of the value is reinterpreted as an object 
+representation in the new type as described in 6.2.6 (a process sometimes 
+called ‘‘type punning’’). This might be a trap representation.
+}}}
+but your mileage may vary in C++, almost all compilers support it, but the spec doesn't allow it. Right
+after this discussion I'll have another solution with memcpy that may be slightly less efficient, (but
+probably not), and is supported by both C and C++):
+
+==== Union version. Fixed for C but not guaranteed portable to C++ ====
 
 {{{
@@ -618 +639 @@
 swaphalves(uint32_t a)
 {
-    union swapem{
-        uint32_t a;
-        uint16_t b[2];
-    };
+    typedef union {
+        uint32_t as32bit;
+        uint16_t as16bit[2];
+    } swapem;
+        
     swapem s={a};
     uint16_t tmp;
-    tmp=s.b[0];
-    s.b[0]=s.b[1];
-    s.b[1]=tmp;
-    return s.a;
+    tmp=s.as16bit[0];
+    s.as16bit[0]=s.as16bit[1];
+    s.as16bit[1]=tmp;
+    return s.as32bit;
 }
 }}}
@@ -633 +655 @@
 {{{
 _Z10swaphalvesj:
-    pushl   %ebp                        # save the original value of ebp
+    pushl   %ebp                # save the original value of ebp
     movl    %esp, %ebp          # point ebp at the stack frame
     movl    8(%ebp), %eax       # get a in eax
-    popl    %ebp                        # get the original ebp value back
+    popl    %ebp                # get the original ebp value back
     roll    $16, %eax           # swap the two halves of a and return it
     ret
 }}}
-
+        So do it wrong, via strange casts and get incorrect code, or by turning off strict-aliasing get inefficient code, or do it right and get efficient code.
+
+        You can also accomplish the same thing by using memcpy with char* to move the data around for the swap, and it will probably be as efficient.  Wait, you ask me, how can that be?  The will be at least to calls to memcpy added to the mix!  Well gcc and other modern compilers have smart optimizers and will, in many cases, (including this one), elide the calls to memcpy.  That makes it the most portable, and as efficient as any other method.  Here's how it would look:
+
+==== memcpy version, compliant to C and C++ specs and efficient ====
+
+{{{
+uint32_t
+swaphalves(uint32_t a)
+{
+    uint16_t as16bit[2],tmp;
+
+    memcpy(as16bit, &a, sizeof(a));
+    tmp = as16bit[0];
+    as16bit[0] = as16bit[1];
+    as16bit[1] = tmp;
+    memcpy(&a, as16bit, sizeof(a));
+    return a;
+}
+}}}
+        For the above code, a C compiler will generate code similar to the previous solution, but with the addition of two calls to memcpy (possibly optimized out).  gcc generates code identical to the previous solution.  You can imagine other variants that substitute reading and writing through a char pointer locally for the calls to memcpy.
 
 === Suppressing Warnings in GCC ===