Re: [RFC] bug 28684

[Richard Guenther <rguenther at suse dot de>]

On Mon, 25 Sep 2006, Revital1 Eres wrote:

> 
> Hello,
> 
> Attached is an initial implementation of the -freorder-fp flag described in
> bug 28684
> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28684).
> The new flag should allow optimizations for floating-point arithmetic that
> require
> reordering floating-point operations (eg. a/b --> a*(1/b) or a+b+c -->
> a+c+b).

I'd like to have a/b -> a*(1/b) and similar transformations not under
-freorder-fp, but -freciprocal (or whatever names we want).


@@ -971,7 +971,7 @@

     case MINUS_EXPR:
       /* We can't turn -(A-B) into B-A when we honor signed zeros.  */
-      return (! FLOAT_TYPE_P (type) || flag_unsafe_math_optimizations)
+      return (! FLOAT_TYPE_P (type) || flag_reorder_fp)
             && reorder_operands_p (TREE_OPERAND (t, 0),
                                    TREE_OPERAND (t, 1));

I think this is wrong.  At least the comment suggests it should be
!HONOR_SIGNED_ZEROS (TYPE_MODE (type)).  Same for

@@ -1083,7 +1083,7 @@
       return TREE_OPERAND (t, 0);

     case PLUS_EXPR:
-      if (! FLOAT_TYPE_P (type) || flag_unsafe_math_optimizations)
+      if (! FLOAT_TYPE_P (type) || flag_reorder_fp)
        {
          /* -(A + B) -> (-B) - A.  */
          if (negate_expr_p (TREE_OPERAND (t, 1))
@@ -1107,7 +1107,7 @@

     case MINUS_EXPR:
       /* - (A - B) -> B - A  */
-      if ((! FLOAT_TYPE_P (type) || flag_unsafe_math_optimizations)
+      if ((! FLOAT_TYPE_P (type) || flag_reorder_fp)
          && reorder_operands_p (TREE_OPERAND (t, 0), TREE_OPERAND (t, 
1)))
        return fold_build2 (MINUS_EXPR, type,
                            TREE_OPERAND (t, 1), TREE_OPERAND (t, 0));

etc.  Now I think we should not honor signed zeros, if -freorder-fp
(or -fassociative-math which I would prefer) is in effect, but change
that in flags.h:

/* Like HONOR_NANS, but true if the given mode distinguishes between
   positive and negative zero, and the sign of zero is important.  */
#define HONOR_SIGNED_ZEROS(MODE) \ 
  (MODE_HAS_SIGNED_ZEROS (MODE) && !flag_unsafe_math_optimizations)

(it also suggests to make flag_reorder_fp true if 
flag_unsafe_math_optimization is set)

@@ -1899,7 +1899,7 @@
     }
   
   /* CHECKME: check for !flag_finite_math_only too?  */
-  if (SCALAR_FLOAT_TYPE_P (type) && !flag_unsafe_math_optimizations)
+  if (SCALAR_FLOAT_TYPE_P (type) && !flag_reorder_fp)
     {
       /* Changing the order of operations changes the semantics.  */
       if (vect_print_dump_info (REPORT_DETAILS))

remove the CHECKME comment and place a comment there why 
(olny) flag_reorder_fp is required.

Index: loop-unroll.c
===================================================================
--- loop-unroll.c       (revision 117077)
+++ loop-unroll.c       (working copy)
@@ -1601,7 +1601,7 @@
   mode2 = GET_MODE (XEXP (src, 1));
   if ((FLOAT_MODE_P (mode1)
        || FLOAT_MODE_P (mode2))
-      && !flag_unsafe_math_optimizations)
+      && !flag_reorder_fp)
     return NULL;

   /* Record the accumulator to expand.  */

Likewise.

Index: simplify-rtx.c
===================================================================
--- simplify-rtx.c      (revision 117077)
+++ simplify-rtx.c      (working copy)
@@ -1956,7 +1956,7 @@
       /* Reassociate multiplication, but for floating point MULTs
         only when the user specifies unsafe math optimizations.  */
       if (! FLOAT_MODE_P (mode)
-         || flag_unsafe_math_optimizations)
+         || flag_reorder_fp)
        {
          tem = simplify_associative_operation (code, mode, op0, op1);
          if (tem)
@@ -2364,7 +2364,7 @@

              /* Change FP division by a constant into multiplication.
                 Only do this with -funsafe-math-optimizations.  */
-             if (flag_unsafe_math_optimizations
+             if (flag_reorder_fp
                  && !REAL_VALUES_EQUAL (d, dconst0))
                {
                  REAL_ARITHMETIC (d, RDIV_EXPR, dconst1, d);

You need to update the comments.

Documentation for the new flag is missing as well.

I suggest you split the patch into patches for -freciprocal-math and
-freorder-fp.

Otherwise, thanks for this work!  It had been on my TODO list as well
for too long time...

Richard.

--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs