[PATCH] "Fix" PR39497, dfp.c is not C

Richard Guenther rguenther@suse.de
Mon Apr 6 12:54:00 GMT 2009


On Mon, 6 Apr 2009, Ben Elliston wrote:

> On Thu, 2009-03-26 at 10:27 +0100, Richard Guenther wrote:
> 
> > You can replace every 2nd line of that file with a memcpy.  With
> > some local mods on a-i branch we now "miscompile" dfp.c and all
> > dfp tests fail.
> > 
> > Thus, if the maintainers of dfp.c want to fix it, fine.  It looked
> > too much excercising for me.
> 
> If this looks OK with you, I will commit it.  Tested with a bootstrap on
> x86_64-linux and no regressions on powerpc-linux, powerpc64-linux and
> x86_64-linux.

Yes, this looks good.  Did you check code generation with/without that
patch (I expect we fold the memcpys to aliased direct assignments even
at the tree level).

Thanks,
Richard.

> Cheers, Ben
> 
> 
> 2009-04-06  Ben Elliston  <bje@au.ibm.com>
> 
>         * dfp.c: Replace type punning assignments with memcpy throughout.
>         * Makefile.in (dfp.o-warn): Remove.
> 
> Index: Makefile.in
> ===================================================================
> --- Makefile.in (revision 145590)
> +++ Makefile.in (working copy)
> @@ -175,8 +175,6 @@
>  # be subject to -Werror:
>  # flex output may yield harmless "no previous prototype" warnings
>  build/gengtype-lex.o-warn = -Wno-error
> -# dfp.c contains many alias violations
> -dfp.o-warn = -fno-strict-aliasing
>  # mips-tfile.c contains -Wcast-qual warnings.
>  mips-tfile.o-warn = -Wno-error
>  
> Index: dfp.c
> ===================================================================
> --- dfp.c       (revision 145590)
> +++ dfp.c       (working copy)
> @@ -1,5 +1,6 @@
>  /* Decimal floating point support.
> -   Copyright (C) 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
> +   Copyright (C) 2005, 2006, 2007, 2008, 2009 Free Software
> +   Foundation, Inc.
>  
>  This file is part of GCC.
>  
> @@ -139,7 +140,7 @@
>    decimal_to_decnumber (r, &dn); 
>    decimal32FromNumber (&d32, &dn, &set);
>  
> -  buf[0] = *(uint32_t *) d32.bytes;
> +  memcpy (&buf[0], d32.bytes, sizeof (uint32_t));
>  }
>  
>  /* Decode an IEEE 754 decimal32 type into a real.  */
> @@ -155,7 +156,7 @@
>    decContextDefault (&set, DEC_INIT_DECIMAL128);
>    set.traps = 0;
>  
> -  *((uint32_t *) d32.bytes) = (uint32_t) buf[0];
> +  memcpy (&d32.bytes, &buf[0], sizeof (uint32_t));
>  
>    decimal32ToNumber (&d32, &dn);
>    decimal_from_decnumber (r, &dn, &set); 
> @@ -179,13 +180,13 @@
>  
>    if (WORDS_BIGENDIAN == FLOAT_WORDS_BIG_ENDIAN)
>      {
> -      buf[0] = *(uint32_t *) &d64.bytes[0];
> -      buf[1] = *(uint32_t *) &d64.bytes[4];
> +      memcpy (&buf[0], &d64.bytes[0], sizeof (uint32_t));
> +      memcpy (&buf[1], &d64.bytes[4], sizeof (uint32_t));
>      }
>    else
>      {
> -      buf[0] = *(uint32_t *) &d64.bytes[4];
> -      buf[1] = *(uint32_t *) &d64.bytes[0];
> +      memcpy (&buf[0], &d64.bytes[4], sizeof (uint32_t));
> +      memcpy (&buf[1], &d64.bytes[0], sizeof (uint32_t));
>      }
>  }
>  
> @@ -204,13 +205,13 @@
>  
>    if (WORDS_BIGENDIAN == FLOAT_WORDS_BIG_ENDIAN)
>      {
> -      *((uint32_t *) &d64.bytes[0]) = (uint32_t) buf[0];
> -      *((uint32_t *) &d64.bytes[4]) = (uint32_t) buf[1];
> +      memcpy (&d64.bytes[0], &buf[0], sizeof (uint32_t));
> +      memcpy (&d64.bytes[4], &buf[1], sizeof (uint32_t));
>      }
>    else
>      {
> -      *((uint32_t *) &d64.bytes[0]) = (uint32_t) buf[1];
> -      *((uint32_t *) &d64.bytes[4]) = (uint32_t) buf[0];
> +      memcpy (&d64.bytes[0], &buf[1], sizeof (uint32_t));
> +      memcpy (&d64.bytes[4], &buf[0], sizeof (uint32_t));
>      }
>  
>    decimal64ToNumber (&d64, &dn);
> @@ -235,17 +236,17 @@
>  
>    if (WORDS_BIGENDIAN == FLOAT_WORDS_BIG_ENDIAN)
>      {
> -      buf[0] = *(uint32_t *) &d128.bytes[0];
> -      buf[1] = *(uint32_t *) &d128.bytes[4];
> -      buf[2] = *(uint32_t *) &d128.bytes[8];
> -      buf[3] = *(uint32_t *) &d128.bytes[12];
> +      memcpy (&buf[0], &d128.bytes[0], sizeof (uint32_t));
> +      memcpy (&buf[1], &d128.bytes[4], sizeof (uint32_t));
> +      memcpy (&buf[2], &d128.bytes[8], sizeof (uint32_t));
> +      memcpy (&buf[3], &d128.bytes[12], sizeof (uint32_t));
>      }
>    else
>      {
> -      buf[0] = *(uint32_t *) &d128.bytes[12];
> -      buf[1] = *(uint32_t *) &d128.bytes[8];
> -      buf[2] = *(uint32_t *) &d128.bytes[4];
> -      buf[3] = *(uint32_t *) &d128.bytes[0];
> +      memcpy (&buf[0], &d128.bytes[12], sizeof (uint32_t));
> +      memcpy (&buf[1], &d128.bytes[8], sizeof (uint32_t));
> +      memcpy (&buf[2], &d128.bytes[4], sizeof (uint32_t));
> +      memcpy (&buf[3], &d128.bytes[0], sizeof (uint32_t));
>      }
>  }
>  @@ -264,17 +265,17 @@
>  
>    if (WORDS_BIGENDIAN == FLOAT_WORDS_BIG_ENDIAN)
>      {
> -      *((uint32_t *) &d128.bytes[0])  = (uint32_t) buf[0];
> -      *((uint32_t *) &d128.bytes[4])  = (uint32_t) buf[1];
> -      *((uint32_t *) &d128.bytes[8])  = (uint32_t) buf[2];
> -      *((uint32_t *) &d128.bytes[12]) = (uint32_t) buf[3];
> +      memcpy (&d128.bytes[0],  &buf[0], sizeof (uint32_t));
> +      memcpy (&d128.bytes[4],  &buf[1], sizeof (uint32_t));
> +      memcpy (&d128.bytes[8],  &buf[2], sizeof (uint32_t));
> +      memcpy (&d128.bytes[12], &buf[3], sizeof (uint32_t));
>      }
>    else
>      {
> -      *((uint32_t *) &d128.bytes[0])  = (uint32_t) buf[3];
> -      *((uint32_t *) &d128.bytes[4])  = (uint32_t) buf[2];
> -      *((uint32_t *) &d128.bytes[8])  = (uint32_t) buf[1];
> -      *((uint32_t *) &d128.bytes[12]) = (uint32_t) buf[0];
> +      memcpy (&d128.bytes[0],  &buf[3], sizeof (uint32_t));
> +      memcpy (&d128.bytes[4],  &buf[2], sizeof (uint32_t));
> +      memcpy (&d128.bytes[8],  &buf[1], sizeof (uint32_t));
> +      memcpy (&d128.bytes[12], &buf[0], sizeof (uint32_t));
>      }
>  
>    decimal128ToNumber (&d128, &dn);
> 
> 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex



More information about the Gcc-patches mailing list