gcc-patches@gcc.gnu.org
[Top] [All Lists]

Re: [PATCH] fix c/39902 for decimal float constant folding

Subject: Re: [PATCH] fix c/39902 for decimal float constant folding
From: Richard Guenther
Date: Fri, 26 Jun 2009 19:50:03 +0200
On Fri, Jun 26, 2009 at 7:33 PM, Janis Johnson<janis187@xxxxxxxxxx> wrote:
> In decimal floating-point arithmetic, trailing zeroes are significant;
> the values 1.DD, 1.0DD, and 1.00DD are all different, and arithmetic on
> each of those values gives a different result.  This means that GCC
> should not perform optimizations like replacing (x * 1) with x when
> the constant is a decimal float value.

Is this really true for real_zerop as well?  I do find it odd that zero doesn't
have a single exact representation in decimal floating-point.

> This patch modifies several functions that check for specific values of
> real constants so they don't handle decimal float values.
>
> Tested with bootstrap and regtest of all languages but Ada on
> powerpc64-linux for -m32/-m64.  OK for trunk and, after testing there,
> on 4.4 and 4.3 branches?  This is not a regression but it does fix
> wrong code.

If the real_zerop change is really warranted the patch is ok for trunk
and the branches.

Thanks,
Richard.


> 2009-06-26  Janis Johnson  <janis187@xxxxxxxxxx>
>
> gcc/
>        PR c/39902
>        * tree.c (real_zerop, real_onep, real_twop, real_minus_onep):
>        Special-case decimal float constants.
>
> gcc/testsuite/
>        PR c/39902
>        * gcc.dg/dfp/pr39902.c: New test.
>
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c  (revision 148858)
> +++ gcc/tree.c  (working copy)
> @@ -1591,7 +1591,8 @@ tree_floor_log2 (const_tree expr)
>          : floor_log2 (low));
>  }
>
> -/* Return 1 if EXPR is the real constant zero.  */
> +/* Return 1 if EXPR is the real constant zero.  Trailing zeroes matter for
> +   decimal float constants, so don't return 1 for them.  */
>
>  int
>  real_zerop (const_tree expr)
> @@ -1599,13 +1600,16 @@ real_zerop (const_tree expr)
>   STRIP_NOPS (expr);
>
>   return ((TREE_CODE (expr) == REAL_CST
> -          && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst0))
> +          && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst0)
> +          && !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr)))))
>          || (TREE_CODE (expr) == COMPLEX_CST
>              && real_zerop (TREE_REALPART (expr))
>              && real_zerop (TREE_IMAGPART (expr))));
>  }
>
> -/* Return 1 if EXPR is the real constant one in real or complex form.  */
> +/* Return 1 if EXPR is the real constant one in real or complex form.
> +   Trailing zeroes matter for decimal float constants, so don't return
> +   1 for them.  */
>
>  int
>  real_onep (const_tree expr)
> @@ -1613,13 +1617,15 @@ real_onep (const_tree expr)
>   STRIP_NOPS (expr);
>
>   return ((TREE_CODE (expr) == REAL_CST
> -          && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst1))
> +          && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst1)
> +          && !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr)))))
>          || (TREE_CODE (expr) == COMPLEX_CST
>              && real_onep (TREE_REALPART (expr))
>              && real_zerop (TREE_IMAGPART (expr))));
>  }
>
> -/* Return 1 if EXPR is the real constant two.  */
> +/* Return 1 if EXPR is the real constant two.  Trailing zeroes matter
> +   for decimal float constants, so don't return 1 for them.  */
>
>  int
>  real_twop (const_tree expr)
> @@ -1627,13 +1633,15 @@ real_twop (const_tree expr)
>   STRIP_NOPS (expr);
>
>   return ((TREE_CODE (expr) == REAL_CST
> -          && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst2))
> +          && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst2)
> +          && !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr)))))
>          || (TREE_CODE (expr) == COMPLEX_CST
>              && real_twop (TREE_REALPART (expr))
>              && real_zerop (TREE_IMAGPART (expr))));
>  }
>
> -/* Return 1 if EXPR is the real constant minus one.  */
> +/* Return 1 if EXPR is the real constant minus one.  Trailing zeroes
> +   matter for decimal float constants, so don't return 1 for them.  */
>
>  int
>  real_minus_onep (const_tree expr)
> @@ -1641,7 +1649,8 @@ real_minus_onep (const_tree expr)
>   STRIP_NOPS (expr);
>
>   return ((TREE_CODE (expr) == REAL_CST
> -          && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconstm1))
> +          && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconstm1)
> +          && !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr)))))
>          || (TREE_CODE (expr) == COMPLEX_CST
>              && real_minus_onep (TREE_REALPART (expr))
>              && real_zerop (TREE_IMAGPART (expr))));
> Index: gcc/testsuite/gcc.dg/dfp/pr39902.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/dfp/pr39902.c  (revision 0)
> +++ gcc/testsuite/gcc.dg/dfp/pr39902.c  (revision 0)
> @@ -0,0 +1,239 @@
> +/* { dg-options "--std=gnu99" } */
> +
> +/* Check that optimizations like (x * 1) to x, or (x * -1) to -x,
> +   do not apply to decimal float computations where trailing zeroes
> +   are significant.  */
> +
> +extern void abort (void);
> +int failcnt;
> +
> +#ifdef DBG
> +extern int printf (const char *, ...);
> +#define FAILURE { printf ("failed at line %d\n", __LINE__); failcnt++; }
> +#else
> +#define FAILURE abort ();
> +#endif
> +
> +#define COMPARE32(A,B) \
> +  A.i == B.i
> +
> +#define COMPARE64(A,B) \
> +  A.i[0] == B.i[0] && A.i[1] == B.i[1]
> +
> +#define COMPARE128(A,B) \
> +  A.i[0] == B.i[0] && A.i[1] == B.i[1] && A.i[2] == B.i[2] && A.i[3] == 
> B.i[3]
> +
> +typedef union {
> +  _Decimal32 d;
> +  unsigned int i;
> +} u32;
> +
> +typedef union {
> +  _Decimal64 d;
> +  unsigned int i[2];
> +} u64;
> +
> +typedef union {
> +  _Decimal128 d;
> +  unsigned int i[4];
> +} u128;
> +
> +volatile u32 p32_1;
> +volatile u32 p32_1_0;
> +volatile u32 p32_2_0;
> +volatile u32 m32_1;
> +volatile u32 m32_1_0;
> +volatile u32 m32_2_0;
> +volatile u32 a32;
> +
> +volatile u64 p64_1;
> +volatile u64 p64_1_0;
> +volatile u64 p64_2_0;
> +volatile u64 m64_1;
> +volatile u64 m64_1_0;
> +volatile u64 m64_2_0;
> +volatile u64 a64;
> +
> +volatile u128 p128_1;
> +volatile u128 p128_1_0;
> +volatile u128 p128_2_0;
> +volatile u128 m128_1;
> +volatile u128 m128_1_0;
> +volatile u128 m128_2_0;
> +volatile u128 a128;
> +
> +void
> +init32 (void)
> +{
> +  p32_1.d = 1.DF;
> +  p32_1_0.d = 1.0DF;
> +  p32_2_0.d = 2.0DF;
> +  m32_1.d = -1.DF;
> +  m32_1_0.d = -1.0DF;
> +  m32_2_0.d = -2.0DF;
> +}
> +
> +void
> +init64 (void)
> +{
> +  p64_1.d = 1.DD;
> +  p64_1_0.d = 1.0DD;
> +  p64_2_0.d = 2.0DD;
> +  m64_1.d = -1.DD;
> +  m64_1_0.d = -1.0DD;
> +  m64_2_0.d = -2.0DD;
> +}
> +
> +void
> +init128 (void)
> +{
> +  p128_1.d = 1.DL;
> +  p128_1_0.d = 1.0DL;
> +  p128_2_0.d = 2.0DL;
> +  m128_1.d = -1.DL;
> +  m128_1_0.d = -1.0DL;
> +  m128_2_0.d = -2.0DL;
> +}
> +
> +void
> +doit32 (void)
> +{
> +  /* Multiplying by a value with no trailing zero should not change the
> +     quantum exponent.  */
> +
> +  a32.d = p32_2_0.d * p32_1.d;
> +  if (! (COMPARE32 (a32, p32_2_0)))
> +    FAILURE
> +
> +  a32.d = p32_2_0.d * 1.DF;
> +  if (! (COMPARE32 (a32, p32_2_0)))
> +    FAILURE
> +
> +  a32.d = p32_2_0.d * m32_1.d;
> +  if (! (COMPARE32 (a32, m32_2_0)))
> +    FAILURE
> +
> +  a32.d = p32_2_0.d * -1.DF;
> +  if (! (COMPARE32 (a32, m32_2_0)))
> +    FAILURE
> +
> +  /* Multiplying by a value with a trailing zero should change the
> +     quantum exponent.  */
> +
> +  a32.d = p32_2_0.d * p32_1_0.d;
> +  if (COMPARE32 (a32, p32_2_0))
> +    FAILURE
> +
> +  a32.d = p32_2_0.d * 1.0DF;
> +  if (COMPARE32 (a32, p32_2_0))
> +    FAILURE
> +
> +  a32.d = p32_2_0.d * m32_1_0.d;
> +  if (COMPARE32 (a32, m32_2_0))
> +    FAILURE
> +
> +  a32.d = p32_2_0.d * -1.0DF;
> +  if (COMPARE32 (a32, m32_2_0))
> +    FAILURE
> +}
> +
> +void
> +doit64 (void)
> +{
> +  /* Multiplying by a value with no trailing zero should not change the
> +     quantum exponent.  */
> +
> +  a64.d = p64_2_0.d * p64_1.d;
> +  if (! (COMPARE64 (a64, p64_2_0)))
> +    FAILURE
> +
> +  a64.d = p64_2_0.d * 1.DD;
> +  if (! (COMPARE64 (a64, p64_2_0)))
> +    FAILURE
> +
> +  a64.d = p64_2_0.d * m64_1.d;
> +  if (! (COMPARE64 (a64, m64_2_0)))
> +    FAILURE
> +
> +  a64.d = p64_2_0.d * -1.DD;
> +  if (! (COMPARE64 (a64, m64_2_0)))
> +    FAILURE
> +
> +  /* Multiplying by a value with a trailing zero should change the
> +     quantum exponent.  */
> +
> +  a64.d = p64_2_0.d * p64_1_0.d;
> +  if (COMPARE64 (a64, p64_2_0))
> +    FAILURE
> +
> +  a64.d = p64_2_0.d * 1.0DD;
> +  if (COMPARE64 (a64, p64_2_0))
> +    FAILURE
> +
> +  a64.d = p64_2_0.d * m64_1_0.d;
> +  if (COMPARE64 (a64, m64_2_0))
> +    FAILURE
> +
> +  a64.d = p64_2_0.d * -1.0DD;
> +  if (COMPARE64 (a64, m64_2_0))
> +    FAILURE
> +}
> +
> +void
> +doit128 (void)
> +{
> +  /* Multiplying by a value with no trailing zero should not change the
> +     quantum exponent.  */
> +
> +  a128.d = p128_2_0.d * p128_1_0.d;
> +  if (COMPARE128 (a128, p128_2_0))
> +    FAILURE
> +
> +  a128.d = p128_2_0.d * 1.0DD;
> +  if (COMPARE128 (a128, p128_2_0))
> +    FAILURE
> +
> +  a128.d = p128_2_0.d * m128_1_0.d;
> +  if (COMPARE128 (a128, m128_2_0))
> +    FAILURE
> +
> +  a128.d = p128_2_0.d * -1.0DD;
> +  if (COMPARE128 (a128, m128_2_0))
> +    FAILURE
> +
> +  /* Multiplying by a value with a trailing zero should change the
> +     quantum exponent.  */
> +
> +  a128.d = p128_2_0.d * p128_1.d;
> +  if (! (COMPARE128 (a128, p128_2_0)))
> +    FAILURE
> +
> +  a128.d = p128_2_0.d * 1.DD;
> +  if (! (COMPARE128 (a128, p128_2_0)))
> +    FAILURE
> +
> +  a128.d = p128_2_0.d * m128_1.d;
> +  if (! (COMPARE128 (a128, m128_2_0)))
> +    FAILURE
> +
> +  a128.d = p128_2_0.d * -1.DD;
> +  if (! (COMPARE128 (a128, m128_2_0)))
> +    FAILURE
> +}
> +
> +int
> +main (void)
> +{
> +  init32 ();
> +  init64 ();
> +  init128 ();
> +
> +  doit32 ();
> +  doit64 ();
> +  doit128 ();
> +
> +  if (failcnt != 0)
> +    abort ();
> +
> +  return 0;
> +}
>
>
>

<Prev in Thread] Current Thread [Next in Thread>