public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Michael Meissner <meissner@linux.vnet.ibm.com>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Michael Meissner <meissner@linux.vnet.ibm.com>,
	       GCC Patches <gcc-patches@gcc.gnu.org>,
	       David Edelsohn <dje.gcc@gmail.com>,
	       Bill Schmidt <wschmidt@linux.vnet.ibm.com>
Subject: Re: [PATCH], Add support for __builtin_{sqrt,fma}f128 on PowerPC ISA 3.0
Date: Thu, 14 Sep 2017 22:21:00 -0000	[thread overview]
Message-ID: <20170914222051.GA4924@ibm-tiger.the-meissners.org> (raw)
In-Reply-To: <20170914145414.GX8421@gate.crashing.org>

[-- Attachment #1: Type: text/plain, Size: 4409 bytes --]

On Thu, Sep 14, 2017 at 09:54:14AM -0500, Segher Boessenkool wrote:
> On Wed, Sep 13, 2017 at 05:46:00PM -0400, Michael Meissner wrote:
> > This patch adds support on PowerPC ISA 3.0 for the built-in function
> > __builtin_sqrtf128 generating the XSSQRTQP hardware square root instruction and
> > the built-in function __builtin_fmaf128 generating XSMADDQP, XSMSUBQP,
> > XSNMADDQP, and XSNMSUBQP fused multiply-add instructions.
> > 
> > While I was at it, I changed the documentation so that it no longer documents
> > the 'q' built-in functions (to mirror libquadmath) but instead just documented
> > the 'f128' functions that matches glibc 2.26 and the technical report that
> > added the _FloatF128 date.
> > 
> > I changed the tests that used __fabsq to use __fabsf128 instead.
> > 
> > I also added && lp64 to float128-5.c so that it doesn't cause errors when doing
> > the test for a 32-bit target.  This is due to the fact that if you enable
> > hardware IEEE 128-bit floating point, you eventually will need TImode
> > supported, and that is not supported on 32-bit targets.
> > 
> > I did a bootstrap and make check with subversion id 252033 on a little endian
> > power8 system.  The subversion id 252033 is one of the last svn ids that
> > bootstrap without additional patches on the PowerPC.  There were no regressions
> > in this patch, and I verified the 4 new tests were run.  Can I check this patch
> > into the trunk?
> 
> Yes please.  A few trivial things:
> 
> > 	* doc/extend.texi (RS/6000 built-in functions): Document the
> > 	'f128' IEEE 128-bit floating point built-in functions.  Don't
> > 	document the older 'q' versions of the functions. Document the
> > 	built-in IEEE 128-bit floating point square root and fused
> > 	multiply-add built-ins.
> 
> Dot space space.
> 
> > +/* 1 argument IEEE 128-bit floating point functions that require ISA 3.0
> > +   hardware.  We define both a 'q' version for libquadmath compatibility, and a
> > +   'f128' for glibc 2.26.  We didn't need this for FABS/COPYSIGN, since the
> > +   machine independent built-in support already defines the F128 versions,  */
> 
> Dot instead of comma?
> 
> > --- gcc/testsuite/gcc.target/powerpc/float128-5.c	(revision 252730)
> > +++ gcc/testsuite/gcc.target/powerpc/float128-5.c	(working copy)
> > @@ -1,4 +1,4 @@
> > -/* { dg-do compile { target { powerpc*-*-linux* } } } */
> > +/* { dg-do compile { target { powerpc*-*-linux* && lp64 } } } */
> 
> Maybe add a comment why this is -m64 only?

I removed the changes to the documentation that didn't pertain to the functions
I was adding (we can clean that up some other time), and added a -m64 comment.

I checked this into the trunk:

[gcc]
2017-09-14  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* config/rs6000/rs6000-builtin.def (BU_FLOAT128_1_HW): New macros
	to support float128 built-in functions that require the ISA 3.0
	hardware.
	(BU_FLOAT128_3_HW): Likewise.
	(SQRTF128): Add support for the IEEE 128-bit square root and fma
	built-in functions.
	(FMAF128): Likewise.
	(FMAQ): Likewise.
	* config/rs6000/rs6000.c (rs6000_builtin_mask_calculate): Add
	support for built-in functions that need the ISA 3.0 IEEE 128-bit
	floating point instructions.
	(rs6000_invalid_builtin): Likewise.
	(rs6000_builtin_mask_names): Likewise.
	* config/rs6000/rs6000.h (MASK_FLOAT128_HW): Likewise.
	(RS6000_BTM_FLOAT128_HW): Likewise.
	(RS6000_BTM_COMMON): Likewise.
	* config/rs6000/rs6000.md (fma<mode>4_hw): Add a generator
	function.
	* doc/extend.texi (RS/6000 built-in functions): Document the
	'f128' IEEE 128-bit floating point built-in functions. Don't
	document the older 'q' versions of the functions. Document the
	built-in IEEE 128-bit floating point square root and fused
	multiply-add built-ins.

[gcc/testsuite]
2017-09-14  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* gcc.target/powerpc/abs128-1.c: Use __builtin_fabsf128 instead of
	__builtin_fabsq.
	* gcc.target/powerpc/float128-5.c: Use __builtin_fabsf128 instead
	of __builtin_fabsq.  Prevent the test from running on 32-bit.
	* gcc.target/powerpc/float128-fma1.c: New test.
	* gcc.target/powerpc/float128-fma2.c: Likewise.
	* gcc.target/powerpc/float128-sqrt1.c: Likewise.
	* gcc.target/powerpc/float128-sqrt2.c: Likewise.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

[-- Attachment #2: ieee128-patch31b --]
[-- Type: text/plain, Size: 8844 bytes --]

Index: gcc/config/rs6000/rs6000-builtin.def
===================================================================
--- gcc/config/rs6000/rs6000-builtin.def	(revision 252768)
+++ gcc/config/rs6000/rs6000-builtin.def	(working copy)
@@ -667,6 +667,23 @@
 		     | RS6000_BTC_UNARY),                               \
 		    CODE_FOR_ ## ICODE)                 /* ICODE */
 
+/* IEEE 128-bit floating-point builtins that need the ISA 3.0 hardware.  */
+#define BU_FLOAT128_1_HW(ENUM, NAME, ATTR, ICODE)                       \
+  RS6000_BUILTIN_1 (MISC_BUILTIN_ ## ENUM,              /* ENUM */      \
+		    "__builtin_" NAME,                  /* NAME */      \
+		    RS6000_BTM_FLOAT128_HW,             /* MASK */      \
+		    (RS6000_BTC_ ## ATTR                /* ATTR */      \
+		     | RS6000_BTC_UNARY),                               \
+		    CODE_FOR_ ## ICODE)                 /* ICODE */
+
+#define BU_FLOAT128_3_HW(ENUM, NAME, ATTR, ICODE)                       \
+  RS6000_BUILTIN_3 (MISC_BUILTIN_ ## ENUM,              /* ENUM */      \
+		    "__builtin_" NAME,                  /* NAME */      \
+		    RS6000_BTM_FLOAT128_HW,             /* MASK */      \
+		    (RS6000_BTC_ ## ATTR                /* ATTR */      \
+		     | RS6000_BTC_TERNARY),                             \
+		    CODE_FOR_ ## ICODE)                 /* ICODE */
+
 /* Miscellaneous builtins for instructions added in ISA 3.0.  These
    instructions don't require either the DFP or VSX options, just the basic
    ISA 3.0 enablement since they operate on general purpose registers.  */
@@ -2323,11 +2340,18 @@ BU_P9_OVERLOAD_2 (CMPRB,	"byte_in_range"
 BU_P9_OVERLOAD_2 (CMPRB2,	"byte_in_either_range")
 BU_P9_OVERLOAD_2 (CMPEQB,	"byte_in_set")
 
-/* 1 argument IEEE 128-bit floating-point functions.  */
+/* 1 and 2 argument IEEE 128-bit floating-point functions.  These functions use
+   the older 'q' suffix from libquadmath.  The standard built-in functions
+   support fabsf128 and copysignf128, but older code used these 'q' versions,
+   so keep them around.  */
 BU_FLOAT128_1 (FABSQ,		"fabsq",       CONST, abskf2)
-
-/* 2 argument IEEE 128-bit floating-point functions.  */
 BU_FLOAT128_2 (COPYSIGNQ,	"copysignq",   CONST, copysignkf3)
+
+/* 1 and 3 argument IEEE 128-bit floating point functions that require ISA 3.0
+   hardware.  These functions use the new 'f128' suffix.  Eventually these
+   should be folded into the common built-in function handling. */
+BU_FLOAT128_1_HW (SQRTF128,	"sqrtf128",	CONST, sqrtkf2)
+BU_FLOAT128_3_HW (FMAF128,	"fmaf128",	CONST, fmakf4_hw)
 \f
 /* 1 argument crypto functions.  */
 BU_CRYPTO_1 (VSBOX,		"vsbox",	  CONST, crypto_vsbox)
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 252768)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -3903,7 +3903,8 @@ rs6000_builtin_mask_calculate (void)
 	  | ((TARGET_DFP)		    ? RS6000_BTM_DFP	   : 0)
 	  | ((TARGET_HARD_FLOAT)	    ? RS6000_BTM_HARD_FLOAT : 0)
 	  | ((TARGET_LONG_DOUBLE_128)	    ? RS6000_BTM_LDBL128   : 0)
-	  | ((TARGET_FLOAT128_TYPE)	    ? RS6000_BTM_FLOAT128  : 0));
+	  | ((TARGET_FLOAT128_TYPE)	    ? RS6000_BTM_FLOAT128  : 0)
+	  | ((TARGET_FLOAT128_HW)	    ? RS6000_BTM_FLOAT128_HW : 0));
 }
 
 /* Implement TARGET_MD_ASM_ADJUST.  All asm statements are considered
@@ -16107,6 +16108,9 @@ rs6000_invalid_builtin (enum rs6000_buil
   else if ((fnmask & RS6000_BTM_HARD_FLOAT) != 0)
     error ("builtin function %qs requires the %qs option", name,
 	   "-mhard-float");
+  else if ((fnmask & RS6000_BTM_FLOAT128_HW) != 0)
+    error ("builtin function %qs requires ISA 3.0 IEEE 128-bit floating point",
+	   name);
   else if ((fnmask & RS6000_BTM_FLOAT128) != 0)
     error ("builtin function %qs requires the %qs option", name, "-mfloat128");
   else
@@ -36227,6 +36231,7 @@ static struct rs6000_opt_mask const rs60
   { "hard-float",	 RS6000_BTM_HARD_FLOAT,	false, false },
   { "long-double-128",	 RS6000_BTM_LDBL128,	false, false },
   { "float128",		 RS6000_BTM_FLOAT128,   false, false },
+  { "float128-hw",	 RS6000_BTM_FLOAT128_HW,false, false },
 };
 
 /* Option variables that we want to support inside attribute((target)) and
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 252768)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -640,6 +640,7 @@ extern int rs6000_vector_align[];
 #define MASK_DLMZB			OPTION_MASK_DLMZB
 #define MASK_EABI			OPTION_MASK_EABI
 #define MASK_FLOAT128_KEYWORD		OPTION_MASK_FLOAT128_KEYWORD
+#define MASK_FLOAT128_HW		OPTION_MASK_FLOAT128_HW
 #define MASK_FPRND			OPTION_MASK_FPRND
 #define MASK_P8_FUSION			OPTION_MASK_P8_FUSION
 #define MASK_HARD_FLOAT			OPTION_MASK_HARD_FLOAT
@@ -2499,6 +2500,7 @@ extern int frame_pointer_needed;
 #define RS6000_BTM_LDBL128	MASK_MULTIPLE	/* 128-bit long double.  */
 #define RS6000_BTM_64BIT	MASK_64BIT	/* 64-bit addressing.  */
 #define RS6000_BTM_FLOAT128	MASK_FLOAT128_KEYWORD /* IEEE 128-bit float.  */
+#define RS6000_BTM_FLOAT128_HW	MASK_FLOAT128_HW /* IEEE 128-bit float h/w.  */
 
 #define RS6000_BTM_COMMON	(RS6000_BTM_ALTIVEC			\
 				 | RS6000_BTM_VSX			\
@@ -2517,7 +2519,8 @@ extern int frame_pointer_needed;
 				 | RS6000_BTM_DFP			\
 				 | RS6000_BTM_HARD_FLOAT		\
 				 | RS6000_BTM_LDBL128			\
-				 | RS6000_BTM_FLOAT128)
+				 | RS6000_BTM_FLOAT128			\
+				 | RS6000_BTM_FLOAT128_HW)
 
 /* Define builtin enum index.  */
 
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 252768)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -14316,7 +14316,7 @@ (define_insn "*nabs<mode>2_hw"
    (set_attr "size" "128")])
 
 ;; Initially don't worry about doing fusion
-(define_insn "*fma<mode>4_hw"
+(define_insn "fma<mode>4_hw"
   [(set (match_operand:IEEE128 0 "altivec_register_operand" "=v")
 	(fma:IEEE128
 	 (match_operand:IEEE128 1 "altivec_register_operand" "%v")
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 252768)
+++ gcc/doc/extend.texi	(working copy)
@@ -15339,6 +15339,21 @@ Similar to @code{__builtin_nans}, except
 @findex __builtin_nansq
 @end table
 
+The following built-in functions are available on Linux 64-bit systems
+that use the ISA 3.0 instruction set.
+
+@table @code
+@item __float128 __builtin_sqrtf128 (__float128)
+Similar to @code{__builtin_sqrtf}, except the return and input types
+are @code{__float128}.
+@findex __builtin_sqrtf128
+
+@item __float128 __builtin_fmaf128 (__float128, __float128, __float128)
+Similar to @code{__builtin_fma}, except the return and input types are
+@code{__float128}.
+@findex __builtin_fmaf128
+@end table
+
 The following built-in functions are available for the PowerPC family
 of processors, starting with ISA 2.05 or later (@option{-mcpu=power6}
 or @option{-mcmpb}):
Index: gcc/testsuite/gcc.target/powerpc/abs128-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/abs128-1.c	(revision 252768)
+++ gcc/testsuite/gcc.target/powerpc/abs128-1.c	(working copy)
@@ -39,7 +39,7 @@ main (int argc, int *argv[])
   x.nan.mant_high = 0x1234;
   x.nan.mant_low = 0xabcdef;
 
-  z.value = __builtin_fabsq (x.value);
+  z.value = __builtin_fabsf128 (x.value);
 
   if (z.nan.negative != 0
       || z.nan.exponent != 0x22
@@ -48,7 +48,7 @@ main (int argc, int *argv[])
       || z.nan.mant_low != 0xabcdef)
     abort ();
 
-  z.value = __builtin_fabsq (z.value);
+  z.value = __builtin_fabsf128 (z.value);
 
   if (z.nan.negative != 0
       || z.nan.exponent != 0x22
Index: gcc/testsuite/gcc.target/powerpc/float128-5.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/float128-5.c	(revision 252768)
+++ gcc/testsuite/gcc.target/powerpc/float128-5.c	(working copy)
@@ -1,9 +1,11 @@
-/* { dg-do compile { target { powerpc*-*-linux* } } } */
+/* { dg-do compile { target { powerpc*-*-linux* && lp64 } } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-options "-O2 -mpower9-vector -mno-float128" } */
 
 /* Test that we can use #pragma GCC target to enable -mfloat128 and generate
-   code on ISA 3.0 for the float128 built-in functions.  */
+   code on ISA 3.0 for the float128 built-in functions.  Lp64 is required
+   because we need TImode to be available to enable __float128 using hardware
+   instructions.  */
 
 #ifdef __FLOAT128__
 #error "-mno-float128 should disable initially defining __FLOAT128__"
@@ -18,7 +20,7 @@
 __float128
 qabs (__float128 a)
 {
-  return __builtin_fabsq (a);
+  return __builtin_fabsf128 (a);
 }
 
 /* { dg-final { scan-assembler "xsabsqp"  } } */

      reply	other threads:[~2017-09-14 22:21 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-13 21:46 Michael Meissner
2017-09-13 22:49 ` Joseph Myers
2017-09-14 19:02   ` Michael Meissner
2017-10-19 22:15   ` [PATCH, version 2], Add support for _Float<N> and _Float<N>X sqrt, fma, fmin, fmax built-in functions Michael Meissner
2017-10-19 23:00     ` Joseph Myers
2017-10-19 23:00       ` Michael Meissner
2017-10-19 23:08         ` Joseph Myers
2017-10-24 22:40       ` [PATCH, version 3], " Michael Meissner
2017-10-24 23:13         ` Joseph Myers
2017-10-24 23:58           ` Michael Meissner
2017-10-25 19:25           ` [PATCH, version 4], " Michael Meissner
2017-10-25 20:01             ` Michael Meissner
2017-10-25 20:11             ` Joseph Myers
2017-10-25 20:31               ` Michael Meissner
2017-10-25 20:50                 ` Joseph Myers
2017-10-25 23:37                   ` [PATCH, version 5], " Michael Meissner
2017-10-25 23:56                     ` Joseph Myers
2017-10-26  0:04                       ` [PATCH, version 5a], " Michael Meissner
2017-10-26  0:17                         ` Michael Meissner
2017-10-30 16:29                           ` Joseph Myers
2017-10-30 18:05                             ` Michael Meissner
2017-10-30 18:38                               ` Joseph Myers
2017-10-30 23:06                                 ` Michael Meissner
2017-10-30 23:29                                   ` Michael Meissner
2017-10-30 23:35                                   ` Joseph Myers
2017-10-30 23:48                                     ` Michael Meissner
2017-10-31 18:04                                     ` Michael Meissner
2017-10-31 18:26                                       ` Joseph Myers
2017-10-27  2:27                         ` Segher Boessenkool
2017-09-14 14:54 ` [PATCH], Add support for __builtin_{sqrt,fma}f128 on PowerPC ISA 3.0 Segher Boessenkool
2017-09-14 22:21   ` Michael Meissner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170914222051.GA4924@ibm-tiger.the-meissners.org \
    --to=meissner@linux.vnet.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=segher@kernel.crashing.org \
    --cc=wschmidt@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).