diff mbox

[rs6000,generic,builtins] Fix unary TDmode patterns and add DFP ABS builtins

Message ID 1376672816.5807.8.camel@otta
State New
Headers show

Commit Message

Peter Bergner Aug. 16, 2013, 5:06 p.m. UTC 
The following patch fixes a bug in the rs6000 DFP handling of some _Decimal128
patterns, namely the unary patterns neg, abs and nabs.  These patterns use
the legacy binary floating point instructions fneg, fabs and fnabs to
twiddle the signbit.  However, these instructions only operate on the first
register in the even/odd register pair, so for destiniation and source
operands that do not overlap, the destination's odd register will be left
uninitialized.  This patch fixes that.

I also noticed there is no current way for the ABS and NABS patterns to be
generated, so I created some generic builtins so they can be.  I have also
created testcases to verify we generate correct code.

This bootstrapped and regtested with no regressions.  Is this ok for trunk?

David, since this is a bad code generation bug, is it ok to backport just
the *negtd2_fpr portion of the patch to the open release branches?

Peter


gcc/
	* builtins.def (BUILT_IN_FABSD32): New DFP ABS builtin.
	(BUILT_IN_FABSD64): Likewise.
	(BUILT_IN_FABSD128): Likewise.
	* builtins.c (expand_builtin): Add support for new DFP ABS builtins.
	(fold_builtin_1): Likewise.
	* config/rs6000/dfp.md (*negtd2_fpr): Handle non-overlapping destination
	and source operands.
	(*abstd2_fpr): Likewise.
	(*nabstd2_fpr): Likewise.

gcc/testsuite/
	* gcc.target/powerpc/dfp-dd-2.c: New test.
	* gcc.target/powerpc/dfp-td-2.c: Likewise.

Comments

Jakub Jelinek Aug. 16, 2013, 5:11 p.m. UTC | #1 
On Fri, Aug 16, 2013 at 12:06:56PM -0500, Peter Bergner wrote:
> --- gcc/config/rs6000/dfp.md	(revision 201779)
> +++ gcc/config/rs6000/dfp.md	(working copy)
> @@ -135,8 +135,15 @@ (define_insn "*negtd2_fpr"
>    [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
>  	(neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
>    "TARGET_HARD_FLOAT && TARGET_FPRS"
> -  "fneg %0,%1"
> -  [(set_attr "type" "fp")])
> +  "*
> +{
> +  if (REGNO (operands[0]) == REGNO (operands[1]))
> +    return \"fneg %0,%1\";
> +  else
> +    return \"fneg %0,%1\;fmr %L0,%L1\";
> +}"
> +  [(set_attr "type" "fp")
> +   (set_attr "length" "8")])

Is the length right for the firt case though?
I mean, shouldn't the insn have two alternatives:
=d,d 0,d and length 4,8 ?

> @@ -148,15 +155,29 @@ (define_insn "*abstd2_fpr"
>    [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
>  	(abs:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
>    "TARGET_HARD_FLOAT && TARGET_FPRS"
> -  "fabs %0,%1"
> -  [(set_attr "type" "fp")])
> +  "*
> +{
> +  if (REGNO (operands[0]) == REGNO (operands[1]))
> +    return \"fabs %0,%1\";
> +  else
> +    return \"fabs %0,%1\;fmr %L0,%L1\";
> +}"
> +  [(set_attr "type" "fp")
> +   (set_attr "length" "8")])

Ditto here.

	Jakub
Peter Bergner Aug. 16, 2013, 5:47 p.m. UTC | #2 
On Fri, 2013-08-16 at 19:11 +0200, Jakub Jelinek wrote:
> On Fri, Aug 16, 2013 at 12:06:56PM -0500, Peter Bergner wrote:
> > --- gcc/config/rs6000/dfp.md	(revision 201779)
> > +++ gcc/config/rs6000/dfp.md	(working copy)
> > @@ -135,8 +135,15 @@ (define_insn "*negtd2_fpr"
> >    [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
> >  	(neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
> >    "TARGET_HARD_FLOAT && TARGET_FPRS"
> > -  "fneg %0,%1"
> > -  [(set_attr "type" "fp")])
> > +  "*
> > +{
> > +  if (REGNO (operands[0]) == REGNO (operands[1]))
> > +    return \"fneg %0,%1\";
> > +  else
> > +    return \"fneg %0,%1\;fmr %L0,%L1\";
> > +}"
> > +  [(set_attr "type" "fp")
> > +   (set_attr "length" "8")])
> 
> Is the length right for the firt case though?
> I mean, shouldn't the insn have two alternatives:
> =d,d 0,d and length 4,8 ?

To be honest, I basically just cut/pasted the negtf2_internal pattern
without looking too closely, but I agree you are right.  I'll redo the
patch with your suggestion.  negtf2_internal always generates two insns,
so it doesn't need any changes.

Peter
diff mbox

Patch

Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 201779)
+++ gcc/builtins.c	(working copy)
@@ -5865,6 +5865,9 @@  expand_builtin (tree exp, rtx target, rt
   switch (fcode)
     {
     CASE_FLT_FN (BUILT_IN_FABS):
+    case BUILT_IN_FABSD32:
+    case BUILT_IN_FABSD64:
+    case BUILT_IN_FABSD128:
       target = expand_builtin_fabs (exp, target, subtarget);
       if (target)
 	return target;
@@ -10314,6 +10317,9 @@  fold_builtin_1 (location_t loc, tree fnd
       return fold_builtin_strlen (loc, type, arg0);

     CASE_FLT_FN (BUILT_IN_FABS):
+    case BUILT_IN_FABSD32:
+    case BUILT_IN_FABSD64:
+    case BUILT_IN_FABSD128:
       return fold_builtin_fabs (loc, arg0, type);

     case BUILT_IN_ABS:
Index: gcc/builtins.def
===================================================================
--- gcc/builtins.def	(revision 201779)
+++ gcc/builtins.def	(working copy)
@@ -257,6 +257,9 @@  DEF_C99_BUILTIN        (BUILT_IN_EXPM1L,
 DEF_LIB_BUILTIN        (BUILT_IN_FABS, "fabs", BT_FN_DOUBLE_DOUBLE, ATTR_CONST_NOTHROW_LEAF_LIST)
 DEF_C99_C90RES_BUILTIN (BUILT_IN_FABSF, "fabsf", BT_FN_FLOAT_FLOAT, ATTR_CONST_NOTHROW_LEAF_LIST)
 DEF_C99_C90RES_BUILTIN (BUILT_IN_FABSL, "fabsl", BT_FN_LONGDOUBLE_LONGDOUBLE, ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN        (BUILT_IN_FABSD32, "fabsd32", BT_FN_DFLOAT32_DFLOAT32, ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN        (BUILT_IN_FABSD64, "fabsd64", BT_FN_DFLOAT64_DFLOAT64, ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN        (BUILT_IN_FABSD128, "fabsd128", BT_FN_DFLOAT128_DFLOAT128, ATTR_CONST_NOTHROW_LEAF_LIST)
 DEF_C99_BUILTIN        (BUILT_IN_FDIM, "fdim", BT_FN_DOUBLE_DOUBLE_DOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
 DEF_C99_BUILTIN        (BUILT_IN_FDIMF, "fdimf", BT_FN_FLOAT_FLOAT_FLOAT, ATTR_MATHFN_FPROUNDING_ERRNO)
 DEF_C99_BUILTIN        (BUILT_IN_FDIML, "fdiml", BT_FN_LONGDOUBLE_LONGDOUBLE_LONGDOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
Index: gcc/config/rs6000/dfp.md
===================================================================
--- gcc/config/rs6000/dfp.md	(revision 201779)
+++ gcc/config/rs6000/dfp.md	(working copy)
@@ -135,8 +135,15 @@  (define_insn "*negtd2_fpr"
   [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
 	(neg:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
   "TARGET_HARD_FLOAT && TARGET_FPRS"
-  "fneg %0,%1"
-  [(set_attr "type" "fp")])
+  "*
+{
+  if (REGNO (operands[0]) == REGNO (operands[1]))
+    return \"fneg %0,%1\";
+  else
+    return \"fneg %0,%1\;fmr %L0,%L1\";
+}"
+  [(set_attr "type" "fp")
+   (set_attr "length" "8")])

 (define_expand "abstd2"
   [(set (match_operand:TD 0 "gpc_reg_operand" "")
@@ -148,15 +155,29 @@  (define_insn "*abstd2_fpr"
   [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
 	(abs:TD (match_operand:TD 1 "gpc_reg_operand" "d")))]
   "TARGET_HARD_FLOAT && TARGET_FPRS"
-  "fabs %0,%1"
-  [(set_attr "type" "fp")])
+  "*
+{
+  if (REGNO (operands[0]) == REGNO (operands[1]))
+    return \"fabs %0,%1\";
+  else
+    return \"fabs %0,%1\;fmr %L0,%L1\";
+}"
+  [(set_attr "type" "fp")
+   (set_attr "length" "8")])

 (define_insn "*nabstd2_fpr"
   [(set (match_operand:TD 0 "gpc_reg_operand" "=d")
 	(neg:TD (abs:TD (match_operand:TD 1 "gpc_reg_operand" "d"))))]
   "TARGET_HARD_FLOAT && TARGET_FPRS"
-  "fnabs %0,%1"
-  [(set_attr "type" "fp")])
+  "*
+{
+  if (REGNO (operands[0]) == REGNO (operands[1]))
+    return \"fnabs %0,%1\";
+  else
+    return \"fnabs %0,%1\;fmr %L0,%L1\";
+}"
+  [(set_attr "type" "fp")
+   (set_attr "length" "8")])

 ;; Hardware support for decimal floating point operations.

Index: gcc/testsuite/gcc.target/powerpc/dfp-dd-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/dfp-dd-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/dfp-dd-2.c	(revision 0)
@@ -0,0 +1,26 @@ 
+/* Test generation of DFP instructions for POWER6.  */
+/* { dg-do compile { target { powerpc*-*-linux* && powerpc_fprs } } } */
+/* { dg-options "-std=gnu99 -O1 -mcpu=power6" } */
+
+/* { dg-final { scan-assembler-times "fneg" 1 } } */
+/* { dg-final { scan-assembler-times "fabs" 1 } } */
+/* { dg-final { scan-assembler-times "fnabs" 1 } } */
+/* { dg-final { scan-assembler-times "fmr" 0 } } */
+
+_Decimal64
+func1 (_Decimal64 a, _Decimal64 b)
+{
+  return -b;
+}
+
+_Decimal64
+func2 (_Decimal64 a, _Decimal64 b)
+{
+  return __builtin_fabsd64 (b);
+}
+
+_Decimal64
+func3 (_Decimal64 a, _Decimal64 b)
+{
+  return - __builtin_fabsd64 (b);
+}
Index: gcc/testsuite/gcc.target/powerpc/dfp-td-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/dfp-td-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/dfp-td-2.c	(revision 0)
@@ -0,0 +1,26 @@ 
+/* Test generation of DFP instructions for POWER6.  */
+/* { dg-do compile { target { powerpc*-*-linux* && powerpc_fprs } } } */
+/* { dg-options "-std=gnu99 -O1 -mcpu=power6" } */
+
+/* { dg-final { scan-assembler-times "fneg" 1 } } */
+/* { dg-final { scan-assembler-times "fabs" 1 } } */
+/* { dg-final { scan-assembler-times "fnabs" 1 } } */
+/* { dg-final { scan-assembler-times "fmr" 3 } } */
+
+_Decimal128
+func1 (_Decimal128 a, _Decimal128 b)
+{
+  return -b;
+}
+
+_Decimal128
+func2 (_Decimal128 a, _Decimal128 b)
+{
+  return __builtin_fabsd128 (b);
+}
+
+_Decimal128
+func3 (_Decimal128 a, _Decimal128 b)
+{
+  return - __builtin_fabsd128 (b);
+}