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

[Richard Guenther <richard dot guenther at gmail dot com>]

On Thu, Jul 16, 2009 at 1:27 AM, Janis Johnson<janis187@us.ibm.com> wrote:
> On Wed, 2009-07-15 at 23:21 +0200, Richard Guenther wrote:
>> On Wed, Jul 15, 2009 at 11:14 PM, Janis Johnson<janis187@us.ibm.com> wrote:
>> > In http://gcc.gnu.org/ml/gcc-patches/2009-06/msg02044.html, Richard
>> > Guenther approved a version of this patch for trunk and the branches.
>> > The mainline patch works for the 4.4 branch but not 4.3, where there
>> > are additional passes that do constant folding. ?This patch disables
>> > RTL constant folding for binary operations involving decimal float
>> > constants.
>>
>> I'd rather have the trunk bits that fixed it there backported than putting
>> a check on the branch that is not on trunk. ?Why doesn't trunk need the
>> simplify-rtx.c hunk?
>>
>> Richard.
>
> The changes to tree.c are the same as in the trunk. ?In the 4.3
> branch the call to simplify_binary_operation that does this
> particular constant folding comes through dead_libcall_p, a
> function that no longer exists in 4.4 and later.

I see.  So you are comfortably sure all other places that call
simplify-rtx are properly guarded in trunk?  At least I cannot see
how wrong simplifications are avoided in simplify_binary_operation_1
there.  Which would be the better place to stick you check so
that constant folding via dfp.c can still be performed from
simplify_const_binary_operation?

Thanks,
Richard.

> Janis
>
>> > Bootstrapped and regression tested on powerpc64-linux, -m32/-m64; OK
>> > for the 4.3 branch?
>> >
>> > 2009-07-15 ?Janis Johnson ?<janis187@us.ibm.com>
>> >
>> > gcc/
>> > ? ? ? ?PR c/39902
>> > ? ? ? ?* tree.c (real_zerop, real_onep, real_twop, real_minus_onep):
>> > ? ? ? ?Special-case decimal float constants.
>> > ? ? ? ?* simplify-rtx.c (simplify_binary_operation): Skip for decimal float.
>> >
>> > gcc/testsuite/
>> > ? ? ? ?PR c/39902
>> > ? ? ? ?* gcc.dg/dfp/pr39902.c: New test.
>> >
>> > Index: gcc/tree.c
>> > ===================================================================
>> > --- gcc/tree.c ?(revision 148982)
>> > +++ gcc/tree.c ?(working copy)
>> > @@ -1543,7 +1543,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)
>> > @@ -1551,13 +1552,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)
>> > @@ -1565,13 +1569,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)
>> > @@ -1579,13 +1585,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)
>> > @@ -1593,7 +1601,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/simplify-rtx.c
>> > ===================================================================
>> > --- gcc/simplify-rtx.c ?(revision 148982)
>> > +++ gcc/simplify-rtx.c ?(working copy)
>> > @@ -1533,6 +1533,11 @@ simplify_binary_operation (enum rtx_code
>> > ? gcc_assert (GET_RTX_CLASS (code) != RTX_COMPARE);
>> > ? gcc_assert (GET_RTX_CLASS (code) != RTX_COMM_COMPARE);
>> >
>> > + ?/* Do not simplify decimal floating-point operations, for which trailing
>> > + ? ? zeroes in constants are significant. ?*/
>> > + ?if (DECIMAL_FLOAT_MODE_P (mode))
>> > + ? ? return NULL;
>> > +
>> > ? /* Make sure the constant is second. ?*/
>> > ? if (GET_RTX_CLASS (code) == RTX_COMM_ARITH
>> > ? ? ? && swap_commutative_operands_p (op0, op1))
>> > 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;
>> > +}
>> >
>> >
>> >
>
>