[Bug middle-end/19988] [4.0/4.1 Regression] pessimizes fp multiply-add/subtract combo

[Bug middle-end/19988] [4.0/4.1 Regression] pessimizes fp multiply-add/subtract combo

[mmitchel at gcc dot gnu dot org]

------- Comment #9 from mmitchel at gcc dot gnu dot org  2005-10-31 02:42 -------
Marking as P4; I doubt we have any good way of fixing this for 4.1.


-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.0/4.1/4.2 Regression] pessimizes fp multiply-add/subtract combo

[mmitchel at gcc dot gnu dot org]

-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.3                       |4.0.4


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.0/4.1/4.2/4.3 Regression] pessimizes fp multiply-add/subtract combo

[gdr at gcc dot gnu dot org]

------- Comment #10 from gdr at gcc dot gnu dot org  2007-01-18 03:36 -------
Won't fix for GCC-4.0.x


-- 

gdr at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.4                       |---


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.0/4.1/4.2/4.3 Regression] pessimizes fp multiply-add/subtract combo

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.1.2


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.0/4.1/4.2/4.3 Regression] pessimizes fp multiply-add/subtract combo

[mmitchel at gcc dot gnu dot org]

-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.1.2                       |4.1.3


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.0/4.1/4.2/4.3 Regression] pessimizes fp multiply-add/subtract combo

[stevenj at alum dot mit dot edu]

------- Comment #11 from stevenj at alum dot mit dot edu  2007-08-01 01:01 -------
Given that there seems to be no progress, or even any prospect of progress, on
Roger's "un-CSE" suggestion, isn't there a simpler fix?

The basic issue here seems to be the abovementioned 2004-02-07 patch identified
by Andrew Pinski:

        Optimize A - B as A + (-B), if B is easily negated.

Here we have a concrete example, which appears in real applications, of how
this can hurt performance: basically, it destroys common subexpressions if A+B
also appears in the code.  Roger opined that A + (-B) seems "more canonical"
and "should help" with other optimizations, but he didn't give any specific
examples where this actually helps performance. Absent such examples, isn't it
better to revert that part of the patch?

I mentioned the case of the PowerPC, where gcc previously (3.4) destroyed an
fma by pulling out C*Y via CSE in X +/- (C*Y).  But gcc 4+ preserves the fmas
(saving one fp op) at the cost of an extra constant load, which hardly seems an
improvement.  A better behavior would be, for architectures with fma, to honor
fma's that appear explicitly in the source code, since CSE of C*Y will never
yield an improvement for X +/- C*Y on those architectures (see above). 
However, even without that fix, the old behavior of leaving X - C*Y as-is seems
preferable. [Even if somehow an extra load is better than an extra multiply on
fma architectures(?), marginally improving gcc for PowerPC etc. at the expense
of x86 seems a poor tradeoff.]

The following patch (to gcc-4.2.1) fixes the problem for me (it removes the
extra constant load and the extra multiplication from the example code I
posted), by just disabling the A - B -> A + (-B) transformation for
floating-point expressions, and hence reverting to the gcc-3.4 behavior in that
case.  

+++ fold-const.c        2007-07-31 20:50:29.512587550 -0400
@@ -9038,11 +9038,8 @@

       /* A - B -> A + (-B) if B is easily negatable.  */
       if (negate_expr_p (arg1)
-         && ((FLOAT_TYPE_P (type)
-               /* Avoid this transformation if B is a positive REAL_CST.  */
-              && (TREE_CODE (arg1) != REAL_CST
-                  ||  REAL_VALUE_NEGATIVE (TREE_REAL_CST (arg1))))
-             || (INTEGRAL_TYPE_P (type) && flag_wrapv && !flag_trapv)))
+         /* !FLOAT_TYPE_P (type) might increase # of fp constants */
+         && (INTEGRAL_TYPE_P (type) && flag_wrapv && !flag_trapv))
        return fold_build2 (PLUS_EXPR, type,
                            fold_convert (type, arg0),
                            fold_convert (type, negate_expr (arg1)));

Better yet, disable it for the integer case too (deleting the transformation
entirely), since this transformation can still destroy common subexpressions
even when you have direct constants:

+++ fold-const.c      2007-07-31 20:57:35.339929690 -0400
@@ -9036,17 +9036,6 @@
          && operand_equal_p (arg0, arg1, 0))
        return fold_convert (type, integer_zero_node);

-      /* A - B -> A + (-B) if B is easily negatable.  */
-      if (negate_expr_p (arg1)
-         && ((FLOAT_TYPE_P (type)
-               /* Avoid this transformation if B is a positive REAL_CST.  */
-              && (TREE_CODE (arg1) != REAL_CST
-                  ||  REAL_VALUE_NEGATIVE (TREE_REAL_CST (arg1))))
-             || (INTEGRAL_TYPE_P (type) && flag_wrapv && !flag_trapv)))
-       return fold_build2 (PLUS_EXPR, type,
-                           fold_convert (type, arg0),
-                           fold_convert (type, negate_expr (arg1)));
-
       /* Try folding difference of addresses.  */
       {
        HOST_WIDE_INT diff;


What is the downside?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.2/4.3/4.4 Regression] pessimizes fp multiply-add/subtract combo

[jsm28 at gcc dot gnu dot org]

------- Comment #12 from jsm28 at gcc dot gnu dot org  2008-07-04 16:49 -------
Closing 4.1 branch.


-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.1/4.2/4.3/4.4 Regression]|[4.2/4.3/4.4 Regression]
                   |pessimizes fp multiply-     |pessimizes fp multiply-
                   |add/subtract combo          |add/subtract combo
   Target Milestone|4.1.3                       |4.2.5


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.2/4.3/4.4 Regression] pessimizes fp multiply-add/subtract combo

[dje at gcc dot gnu dot org]

-- 

dje at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bergner at gcc dot gnu dot
                   |                            |org
           Severity|minor                       |major
           Priority|P4                          |P3


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.2/4.3/4.4 Regression] pessimizes fp multiply-add/subtract combo

[pinskia at gcc dot gnu dot org]

------- Comment #13 from pinskia at gcc dot gnu dot org  2008-10-09 19:03 -------
*** Bug 37783 has been marked as a duplicate of this bug. ***


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rrh at google dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.2/4.3/4.4 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org
 GCC target triplet|i686-*-*                    |
      Known to fail|4.0.4                       |4.0.4 4.4.0
           Priority|P3                          |P2


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.2/4.3/4.4 Regression] pessimizes fp multiply-add/subtract combo

[bonzini at gnu dot org]

------- Comment #14 from bonzini at gnu dot org  2009-02-03 09:49 -------
What about canonicalizing to a *positive* number?


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bonzini at gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.2/4.3/4.4 Regression] pessimizes fp multiply-add/subtract combo

[jakub at gcc dot gnu dot org]

------- Comment #15 from jakub at gcc dot gnu dot org  2009-02-03 09:56 -------
I really think either GCSE should be trying to decrease the number of floating
point constants used by a function by getting rid of constants for which
negated constants is also used and it can be negated at no cost (by swapping an
addition with subtraction etc.).  Or if not GCSE, some late tree pass.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[jsm28 at gcc dot gnu dot org]

------- Comment #16 from jsm28 at gcc dot gnu dot org  2009-03-31 16:45 -------
Closing 4.2 branch.


-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.2/4.3/4.4/4.5 Regression]|[4.3/4.4/4.5 Regression]
                   |pessimizes fp multiply-     |pessimizes fp multiply-
                   |add/subtract combo          |add/subtract combo
   Target Milestone|4.2.5                       |4.3.4


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

------- Comment #17 from rguenth at gcc dot gnu dot org  2009-08-04 12:26 -------
GCC 4.3.4 is being released, adjusting target milestone.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.3.4                       |4.3.5


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

------- Comment #18 from rguenth at gcc dot gnu dot org  2010-01-16 19:09 -------
Created an attachment (id=19626)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19626&action=view)
VN based fix

This extends the value-numberers value insertion facility to handle this
special case.  Well, it needs TLC obviously - it was just a quick hack.

We now end up with

<bb 2>:
  D.2649_2 = y_1(D) * 1.23399999999999995803356966916908277198672294617e-1;
  D.2650_4 = D.2649_2 + x_3(D);
  pretmp.1_9 = -D.2649_2;
  D.2652_6 = x_3(D) + pretmp.1_9;
  D.2648_7 = D.2650_4 * D.2652_6;
  return D.2648_7;

probably the same as 3.3.  Another interesting bit is that we do not ever
merge the negation into the addition on the tree level (we do that only
during combine).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

------- Comment #19 from rguenth at gcc dot gnu dot org  2010-01-16 19:15 -------
What does (can) the new VN do here?


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matz at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[matz at gcc dot gnu dot org]

------- Comment #20 from matz at gcc dot gnu dot org  2010-01-16 20:10 -------
It obviously could do the same.  It all depends on the answer to this
question:
   Do we want to deal with this via canonicalization of expressions, and if
   yes, then also on the outcome of our discussion of where that should happen
   (fold or special code in VN dealing with just value numbers).
If no, then something like your patch can be done.

In this case I'm leaning towards Paolo (comment #14), we possibly might want
to canonicalize towards fewer constants (hence towards positive constants),
when otherwise the number of operations remains the same (hence treating
PLUS_EXPR and MINUS_EXPR as equivalent).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

------- Comment #21 from rguenth at gcc dot gnu dot org  2010-01-16 20:30 -------
(In reply to comment #20)
> It obviously could do the same.  It all depends on the answer to this
> question:
>    Do we want to deal with this via canonicalization of expressions, and if
>    yes, then also on the outcome of our discussion of where that should happen
>    (fold or special code in VN dealing with just value numbers).
> If no, then something like your patch can be done.
> 
> In this case I'm leaning towards Paolo (comment #14), we possibly might want
> to canonicalize towards fewer constants (hence towards positive constants),
> when otherwise the number of operations remains the same (hence treating
> PLUS_EXPR and MINUS_EXPR as equivalent).

Yes, that sounds reasonable.  Note that most existing foldings only look
for PLUS_EXPR once the 2nd operand is a constant, so auditing of them is
required.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

------- Comment #22 from rguenth at gcc dot gnu dot org  2010-01-16 21:42 -------
Created an attachment (id=19627)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19627&action=view)
fixed patch

Fixed patch that actually bootstraps.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #19626|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

------- Comment #23 from rguenth at gcc dot gnu dot org  2010-01-18 15:16 -------
And a fix along comment #14 would be (untested, but of course fixes the
testcase):

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 156009)
+++ gcc/fold-const.c    (working copy)
@@ -1129,10 +1129,14 @@ negate_expr_p (tree t)
              && TYPE_OVERFLOW_WRAPS (type));

     case FIXED_CST:
-    case REAL_CST:
     case NEGATE_EXPR:
       return true;

+    case REAL_CST:
+      /* We want to canonicalize to positive real constants.  Pretend
+         that only negative ones can be easily negated.  */
+      return REAL_VALUE_NEGATIVE (TREE_REAL_CST (t));
+
     case COMPLEX_CST:
       return negate_expr_p (TREE_REALPART (t))
             && negate_expr_p (TREE_IMAGPART (t));


looks appealing, but let's check for fallout.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|42802                       |
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2005-08-17 12:43:49         |2010-01-19 11:38:04
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4/4.5 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

------- Comment #24 from rguenth at gcc dot gnu dot org  2010-01-21 12:15 -------
Subject: Bug 19988

Author: rguenth
Date: Thu Jan 21 12:15:40 2010
New Revision: 156152

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156152
Log:
2010-01-21  Richard Guenther  <rguenther@suse.de>

        PR middle-end/19988
        * fold-const.c (negate_expr_p): Pretend only negative
        real constants are easily negatable.

        * gcc.dg/pr19988.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/pr19988.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988

[Bug middle-end/19988] [4.3/4.4 Regression] pessimizes fp multiply-add/subtract combo

[rguenth at gcc dot gnu dot org]

------- Comment #25 from rguenth at gcc dot gnu dot org  2010-01-21 12:16 -------
Fixed for 4.5.  Will not fix in earlier releases due to possible regressions
the patch will cause.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |4.5.0
         Resolution|                            |FIXED
            Summary|[4.3/4.4/4.5 Regression]    |[4.3/4.4 Regression]
                   |pessimizes fp multiply-     |pessimizes fp multiply-
                   |add/subtract combo          |add/subtract combo
   Target Milestone|4.3.5                       |4.5.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19988