diff mbox

Fix PR55152

Message ID alpine.LSU.2.11.1610051324150.26629@t29.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Oct. 5, 2016, 11:24 a.m. UTC 
On Wed, 5 Oct 2016, Richard Biener wrote:

> On Tue, 4 Oct 2016, Joseph Myers wrote:
> 
> > On Tue, 4 Oct 2016, Richard Biener wrote:
> > 
> > > On Tue, 4 Oct 2016, Joseph Myers wrote:
> > > 
> > > > On Tue, 4 Oct 2016, Richard Biener wrote:
> > > > 
> > > > > Possibly.  Though then for FP we also want - abs (a) -> copysign (a, -1).
> > > > 
> > > > For architectures such as powerpc that have a negated-abs instruction, 
> > > > does it get properly generated from the copysign code?  (The relevant 
> > > > pattern in rs6000.md uses (neg (abs)) - is that the correct canonical RTL 
> > > > for this?)
> > > 
> > > I have no idea - given that there is no copysign RTL code I suppose
> > > it is the only machine independent RTL that can do this?
> > > 
> > > The question would be whether the copysign optab of said targets would
> > > special-case the -1 case appropriately.
> > 
> > Why -1?  Any constant in copysign should be handled appropriately.  I'd 
> > say that (neg (abs)) is probably better as a canonical representation, so 
> > map copysign from a constant to either (abs) or (neg (abs)) appropriately.
> > 
> > Then in the case where abs is expanded with bit manipulation, (neg (abs)) 
> > should be expanded to a single OR.  I don't know whether the RTL 
> > optimizers will map the AND / OR combination to a single OR, but if they 
> > do then there should be no need to special-case (neg (abs)) in expansion, 
> > and when (abs) RTL is generated a machine description's (neg (abs)) 
> > pattern should match automatically.
> 
> Ok, that's a good point -- I'll do copysign (X, const) canonicalization
> to [-]abs() then.  Hopefully that's a valid transform even if NaNs and
> signed zeros are involved.

AFAICS it is.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2016-10-05  Richard Biener  <rguenther@suse.de>

	* match.pd (copysign(x, CST) -> [-]abs (x)): New pattern.

	* gcc.dg/fold-copysign-1.c: New testcase.
diff mbox

Patch

Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 240770)
+++ gcc/match.pd	(working copy)
@@ -452,6 +452,14 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (hypots @0 (copysigns @1 @2))
   (hypots @0 @1)))
 
+/* copysign(x, CST) -> [-]abs (x).  */
+(for copysigns (COPYSIGN)
+ (simplify
+  (copysigns @0 REAL_CST@1)
+  (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1)))
+   (negate (abs @0))
+   (abs @0))))
+
 /* copysign(copysign(x, y), z) -> copysign(x, z).  */
 (for copysigns (COPYSIGN)
  (simplify
Index: gcc/testsuite/gcc.dg/fold-copysign-1.c
===================================================================
--- gcc/testsuite/gcc.dg/fold-copysign-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/fold-copysign-1.c	(working copy)
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-cddce1" } */
+
+double foo (double x)
+{
+  double one = 1.;
+  return __builtin_copysign (x, one);
+}
+double bar (double x)
+{
+  double minuszero = -0.;
+  return __builtin_copysign (x, minuszero);
+}
+
+/* { dg-final { scan-tree-dump-times "= -" 1 "cddce1" } } */
+/* { dg-final { scan-tree-dump-times "= ABS_EXPR" 2 "cddce1" } } */