diff mbox

[rs6000] Fix PR target/80246, DFP builtins using the wrong types

Message ID cb494b8c-cfbc-c481-72c2-a71570b5f2c6@vnet.ibm.com
State New
Headers show

Commit Message

Peter Bergner March 30, 2017, 2:37 p.m. UTC 
On 3/29/17 6:29 PM, Peter Bergner wrote:
> On 3/29/17 5:28 PM, Segher Boessenkool wrote:

>>> +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
>>> +/* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */
>>> +/* { dg-require-effective-target dfp } */
>>
>> You can leave off the default arguments "*" "".  Do we really need to
>> explicitly skip on Darwin and SPE if there is a test for DFP anyway?

Ok, I removed the darwin and spe tests, since the dfp requirement will
disallow those.  I made the same change to teh dfp-builtin-1.c test case
as well.  I also failed to realize that dfp-builtin-1.c was failing due
to the check for no "stfd" and "lfd", which we will now expect to see,
since we're compiling the test case with -mcpu=power7, therefore I
updated the test case to expect them.


>>> +/* { dg-final { scan-assembler-not "drintn\\." } } */
>>> +/* { dg-final { scan-assembler-not "drintnq\\." } } */
>>> +/* { dg-final { scan-assembler-not "dctfix" } } */
>>> +/* { dg-final { scan-assembler-not "dctfixq" } } */
>>
>> If there is no "dctfix" there surely is no "dctfixq" either (i.e., your
>> regexen aren't very tight).
> 
> Ahh, true.  I suppose I could also just look for "drintn" too,
> since that would catch both drintn. and drintnq., ok with that
> change?

Fixed.  Here is an updated patch.  Is this better?

Peter


gcc/
	PR target/80246
	* config/rs6000/dfp.md (dfp_dxex_<mode>): Update mode of operand 0.
	(dfp_diex_<mode>): Update mode of operand 1.
	* doc/extend.texi (dxex, dxexq): Document change to return type.
	(diex, diexq): Document change to argument type.

gcc/testsuite/
	PR target/80246
	* gcc.target/powerpc/dfp-builtin-1.c (dxex, dxexq): Update return types.
	(diex, diexq): Update argument type.
	* gcc.target/powerpc/pr80246.c: New test.

Comments

Andreas Schwab April 2, 2017, 7:21 a.m. UTC | #1 
On Mär 30 2017, Peter Bergner <bergner@vnet.ibm.com> wrote:

> Index: gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c	(revision 246539)
> +++ gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c	(working copy)
> @@ -1,6 +1,4 @@
>  /* { dg-do compile { target { powerpc*-*-linux* } } } */
> -/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
> -/* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */
>  /* { dg-require-effective-target powerpc_vsx_ok } */
>  /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
>  /* { dg-options "-mcpu=power7 -O2" } */
> @@ -10,11 +8,13 @@
>  /* { dg-final { scan-assembler-times "diex "   1    } } */
>  /* { dg-final { scan-assembler-times "dscli "  2    } } */
>  /* { dg-final { scan-assembler-times "dscri "  2    } } */
> +/* { dg-final { scan-assembler-times "std "    1    } } */
> +/* { dg-final { scan-assembler-times "ld "     1    } } */

Fails with -m32.

Andreas.
Andreas Schwab April 2, 2017, 7:29 a.m. UTC | #2 
On Mär 30 2017, Peter Bergner <bergner@vnet.ibm.com> wrote:

> Index: gcc/testsuite/gcc.target/powerpc/pr80246.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/pr80246.c	(nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/pr80246.c	(working copy)
> @@ -0,0 +1,35 @@
> +/* { dg-do compile { target { powerpc*-*-linux* } } } */
> +/* { dg-require-effective-target dfp } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler-times "dxex "  1 } } */
> +/* { dg-final { scan-assembler-times "dxexq " 1 } } */
> +/* { dg-final { scan-assembler-times "diex "  1 } } */
> +/* { dg-final { scan-assembler-times "diexq " 1 } } */
> +/* { dg-final { scan-assembler-not "bl __builtin" } } */
> +/* { dg-final { scan-assembler-not "drintn" } } */
> +/* { dg-final { scan-assembler-not "dctfix" } } */
> +/* { dg-final { scan-assembler-not "dcffix" } } */
> +
> +long long
> +do_xex (_Decimal64 arg)
> +{
> +  return __builtin_dxex (arg);
> +}
> +
> +long long
> +do_xexq (_Decimal128 arg)
> +{
> +  return __builtin_dxexq (arg);
> +}
> +
> +_Decimal64
> +do_iex (long long exp, _Decimal64 arg)
> +{
> +  return __builtin_diex (exp, arg);
> +}
> +
> +_Decimal128
> +do_iexq (long long exp, _Decimal128 arg)
> +{
> +  return __builtin_diexq (exp, arg);
> +}

FAIL: gcc.target/powerpc/pr80246.c (test for excess errors)
Excess errors:
/daten/gcc/gcc-20170401/gcc/testsuite/gcc.target/powerpc/pr80246.c:18:10: error: Builtin function __builtin_dxex requires the -mhard-dfp option
/daten/gcc/gcc-20170401/gcc/testsuite/gcc.target/powerpc/pr80246.c:24:10: error: Builtin function __builtin_dxexq requires the -mhard-dfp option
/daten/gcc/gcc-20170401/gcc/testsuite/gcc.target/powerpc/pr80246.c:30:10: error: Builtin function __builtin_diex requires the -mhard-dfp option
/daten/gcc/gcc-20170401/gcc/testsuite/gcc.target/powerpc/pr80246.c:36:10: error: Builtin function __builtin_diexq requires the -mhard-dfp option

Andreas.
Peter Bergner April 2, 2017, 2:48 p.m. UTC | #3 
On 4/2/17 2:29 AM, Andreas Schwab wrote:
>> +/* { dg-require-effective-target dfp } */
[snip]
> FAIL: gcc.target/powerpc/pr80246.c (test for excess errors)
> Excess errors:
> /daten/gcc/gcc-20170401/gcc/testsuite/gcc.target/powerpc/pr80246.c:18:10:
>  error: Builtin function __builtin_dxex requires the -mhard-dfp option

What configure options are you using?  I would have expected this the
dg-require-effective-target to disable this test if you don't have
-mhard-dfp.

Peter
Andreas Schwab April 2, 2017, 3:07 p.m. UTC | #4 
On Apr 02 2017, Peter Bergner <bergner@vnet.ibm.com> wrote:

> On 4/2/17 2:29 AM, Andreas Schwab wrote:
>>> +/* { dg-require-effective-target dfp } */
> [snip]
>> FAIL: gcc.target/powerpc/pr80246.c (test for excess errors)
>> Excess errors:
>> /daten/gcc/gcc-20170401/gcc/testsuite/gcc.target/powerpc/pr80246.c:18:10:
>>  error: Builtin function __builtin_dxex requires the -mhard-dfp option
>
> What configure options are you using?

http://gcc.gnu.org/ml/gcc-testresults/2017-04/msg00126.html

Andreas.
Segher Boessenkool April 2, 2017, 6:53 p.m. UTC | #5 
On Sun, Apr 02, 2017 at 09:48:36AM -0500, Peter Bergner wrote:
> On 4/2/17 2:29 AM, Andreas Schwab wrote:
> >> +/* { dg-require-effective-target dfp } */
> [snip]
> > FAIL: gcc.target/powerpc/pr80246.c (test for excess errors)
> > Excess errors:
> > /daten/gcc/gcc-20170401/gcc/testsuite/gcc.target/powerpc/pr80246.c:18:10:
> >  error: Builtin function __builtin_dxex requires the -mhard-dfp option
> 
> What configure options are you using?  I would have expected this the
> dg-require-effective-target to disable this test if you don't have
> -mhard-dfp.

This should test hard_dfp instead of dfp.  I also have a fix for the
dfp-builtin-1.c problem.  Still pondering what to do about the last one,
fold-vec-mule-misc.c: vsx_ok is pretty useless, or confusingly named at
least.


Segher
Peter Bergner April 3, 2017, 2:41 p.m. UTC | #6 
On 4/2/17 1:53 PM, Segher Boessenkool wrote:
> On Sun, Apr 02, 2017 at 09:48:36AM -0500, Peter Bergner wrote:
>> On 4/2/17 2:29 AM, Andreas Schwab wrote:
>>>> +/* { dg-require-effective-target dfp } */
>> [snip]
>>> FAIL: gcc.target/powerpc/pr80246.c (test for excess errors)
>>> Excess errors:
>>> /daten/gcc/gcc-20170401/gcc/testsuite/gcc.target/powerpc/pr80246.c:18:10:
>>>  error: Builtin function __builtin_dxex requires the -mhard-dfp option
>>
>> What configure options are you using?  I would have expected this the
>> dg-require-effective-target to disable this test if you don't have
>> -mhard-dfp.
> 
> This should test hard_dfp instead of dfp.  

Ah, yes, dfp is just support for they DFP types.  I'll make that
change and commit it.



> I also have a fix for the dfp-builtin-1.c problem.

You mean you have a patch to the regex to match both std/stw and ld/lwz?



> Still pondering what to do about the last one, fold-vec-mule-misc.c:
> vsx_ok is pretty useless, or confusingly named at least.

Ummm, how would my patch have affected that FAIL, as it doesn't use
any _Decimal* types, let alone the dxex* or diex* patterns that I
changed?

Peter
Peter Bergner April 3, 2017, 2:54 p.m. UTC | #7 
On 4/3/17 9:41 AM, Peter Bergner wrote:
> On 4/2/17 1:53 PM, Segher Boessenkool wrote:
>> I also have a fix for the dfp-builtin-1.c problem.
> 
> You mean you have a patch to the regex to match both std/stw and ld/lwz?

I think we should also add:

  /* { dg-require-effective-target hard_dfp } */

to the dfp-builtin-1.c requirements, since I think someone could do
-mno-hard-dfp while also having VSX enabled.  Ditto for dfp-builtin-2.c.

Peter
diff mbox

Patch

Index: gcc/config/rs6000/dfp.md
===================================================================
--- gcc/config/rs6000/dfp.md	(revision 246539)
+++ gcc/config/rs6000/dfp.md	(working copy)
@@ -348,9 +348,9 @@ 
   [(set_attr "type" "dfp")])
 
 (define_insn "dfp_dxex_<mode>"
-  [(set (match_operand:D64_D128 0 "gpc_reg_operand" "=d")
-	(unspec:D64_D128 [(match_operand:D64_D128 1 "gpc_reg_operand" "d")]
-			 UNSPEC_DXEX))]
+  [(set (match_operand:DI 0 "gpc_reg_operand" "=d")
+	(unspec:DI [(match_operand:D64_D128 1 "gpc_reg_operand" "d")]
+		   UNSPEC_DXEX))]
   "TARGET_DFP"
   "dxex<dfp_suffix> %0,%1"
   [(set_attr "type" "dfp")])
@@ -357,7 +357,7 @@ 
 
 (define_insn "dfp_diex_<mode>"
   [(set (match_operand:D64_D128 0 "gpc_reg_operand" "=d")
-	(unspec:D64_D128 [(match_operand:D64_D128 1 "gpc_reg_operand" "d")
+	(unspec:D64_D128 [(match_operand:DI 1 "gpc_reg_operand" "d")
 			  (match_operand:D64_D128 2 "gpc_reg_operand" "d")]
 			 UNSPEC_DXEX))]
   "TARGET_DFP"
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 246539)
+++ gcc/doc/extend.texi	(working copy)
@@ -15427,14 +15427,14 @@ 
 of processors when hardware decimal floating point
 (@option{-mhard-dfp}) is available:
 @smallexample
-_Decimal64 __builtin_dxex (_Decimal64);
-_Decimal128 __builtin_dxexq (_Decimal128);
+long long __builtin_dxex (_Decimal64);
+long long __builtin_dxexq (_Decimal128);
 _Decimal64 __builtin_ddedpd (int, _Decimal64);
 _Decimal128 __builtin_ddedpdq (int, _Decimal128);
 _Decimal64 __builtin_denbcd (int, _Decimal64);
 _Decimal128 __builtin_denbcdq (int, _Decimal128);
-_Decimal64 __builtin_diex (_Decimal64, _Decimal64);
-_Decimal128 _builtin_diexq (_Decimal128, _Decimal128);
+_Decimal64 __builtin_diex (long long, _Decimal64);
+_Decimal128 _builtin_diexq (long long, _Decimal128);
 _Decimal64 __builtin_dscli (_Decimal64, int);
 _Decimal128 __builtin_dscliq (_Decimal128, int);
 _Decimal64 __builtin_dscri (_Decimal64, int);
Index: gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c	(revision 246539)
+++ gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c	(working copy)
@@ -1,6 +1,4 @@ 
 /* { dg-do compile { target { powerpc*-*-linux* } } } */
-/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
-/* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */
 /* { dg-require-effective-target powerpc_vsx_ok } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
 /* { dg-options "-mcpu=power7 -O2" } */
@@ -10,11 +8,13 @@ 
 /* { dg-final { scan-assembler-times "diex "   1    } } */
 /* { dg-final { scan-assembler-times "dscli "  2    } } */
 /* { dg-final { scan-assembler-times "dscri "  2    } } */
+/* { dg-final { scan-assembler-times "std "    1    } } */
+/* { dg-final { scan-assembler-times "ld "     1    } } */
+/* { dg-final { scan-assembler-times "stfd "   1    } } */
+/* { dg-final { scan-assembler-times "lfd "    1    } } */
 /* { dg-final { scan-assembler-not   "bl __builtin" } } */
 /* { dg-final { scan-assembler-not   "dctqpq"       } } */
 /* { dg-final { scan-assembler-not   "drdpq"        } } */
-/* { dg-final { scan-assembler-not   "stfd"         } } */
-/* { dg-final { scan-assembler-not   "lfd"          } } */
 
 _Decimal64
 do_dedpd_0 (_Decimal64 a)
@@ -52,7 +52,7 @@ 
   return __builtin_denbcd (1, a);
 }
 
-_Decimal64
+long long
 do_xex (_Decimal64 a)
 {
   return __builtin_dxex (a);
@@ -59,7 +59,7 @@ 
 }
 
 _Decimal64
-do_iex (_Decimal64 a, _Decimal64 b)
+do_iex (long long a, _Decimal64 b)
 {
   return __builtin_diex (a, b);
 }
Index: gcc/testsuite/gcc.target/powerpc/pr80246.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80246.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr80246.c	(working copy)
@@ -0,0 +1,35 @@ 
+/* { dg-do compile { target { powerpc*-*-linux* } } } */
+/* { dg-require-effective-target dfp } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-times "dxex "  1 } } */
+/* { dg-final { scan-assembler-times "dxexq " 1 } } */
+/* { dg-final { scan-assembler-times "diex "  1 } } */
+/* { dg-final { scan-assembler-times "diexq " 1 } } */
+/* { dg-final { scan-assembler-not "bl __builtin" } } */
+/* { dg-final { scan-assembler-not "drintn" } } */
+/* { dg-final { scan-assembler-not "dctfix" } } */
+/* { dg-final { scan-assembler-not "dcffix" } } */
+
+long long
+do_xex (_Decimal64 arg)
+{
+  return __builtin_dxex (arg);
+}
+
+long long
+do_xexq (_Decimal128 arg)
+{
+  return __builtin_dxexq (arg);
+}
+
+_Decimal64
+do_iex (long long exp, _Decimal64 arg)
+{
+  return __builtin_diex (exp, arg);
+}
+
+_Decimal128
+do_iexq (long long exp, _Decimal128 arg)
+{
+  return __builtin_diexq (exp, arg);
+}