Re: [PATCH] rs6000: Some builtins require IBM-128 long double format (PR103623)

2021-12-13 Thread Segher Boessenkool
Hi!

On Mon, Dec 13, 2021 at 09:55:17AM -0600, Bill Schmidt wrote:
> PR103623 shows that we ICE if __builtin_pack_longdouble or 
> __builtin_unpack_longdouble
> is used when long double is not defined to be the IBM-128 (double-double) 
> format.
> To solve this, I introduce a new built-in function attribute "ibmld" that 
> enforces
> this requirement.

Good call.

> @@ -215,13 +216,8 @@
>double __builtin_mffsl ();
>  MFFSL rs6000_mffsl {}
>  
> -; This thing really assumes long double == __ibm128, and I'm told it has
> -; been used as such within libgcc.  Given that __builtin_pack_ibm128
> -; exists for the same purpose, this should really not be used at all.
> -; TODO: Consider adding special handling for this to warn whenever
> -; long double is not __ibm128.
>const long double __builtin_pack_longdouble (double, double);
> -PACK_TF packtf {}
> +PACK_TF packtf {ibmld}

You removed a bit much of the comment.  Please retain the part about
__builtin_pack_ibm128 being better to use, less antiquated.

> -; See above comments for __builtin_pack_longdouble.
>const double __builtin_unpack_longdouble (long double, const int<1>);
> -UNPACK_TF unpacktf {}
> +UNPACK_TF unpacktf {ibmld}

And that comment :-)  (Or just copy the short comment from there).

Okay for trunk with that tweaked (and Martin's quoting thing taken care
of).  Thanks!


Segher


Re: [PATCH] rs6000: Some builtins require IBM-128 long double format (PR103623)

2021-12-13 Thread Bill Schmidt via Gcc-patches
Hi!

On 12/13/21 2:15 PM, Martin Sebor wrote:
> On 12/13/21 8:55 AM, Bill Schmidt via Gcc-patches wrote:
>> Hi!
>>
>> PR103623 shows that we ICE if __builtin_pack_longdouble or 
>> __builtin_unpack_longdouble
>> is used when long double is not defined to be the IBM-128 (double-double) 
>> format.
>> To solve this, I introduce a new built-in function attribute "ibmld" that 
>> enforces
>> this requirement.
>>
>> Tested the fix on a powerpc-e300c3-linux-gnu cross.  Bootstrapped and tested 
>> on
>> powerpc64le-linux-gnu with no regressions.  Is this okay for trunk?
>
> Just a minor note about the format of the new error message
> below:
>
> ...
>> diff --git a/gcc/config/rs6000/rs6000-call.c 
>> b/gcc/config/rs6000/rs6000-call.c
>> index d9736eaf21c..b6f0c6c4c08 100644
>> --- a/gcc/config/rs6000/rs6000-call.c
>> +++ b/gcc/config/rs6000/rs6000-call.c
>> @@ -15741,6 +15741,13 @@ rs6000_expand_new_builtin (tree exp, rtx target,
>>     return const0_rtx;
>>   }
>>   +  if (bif_is_ibmld (*bifaddr) && !FLOAT128_2REG_P (TFmode))
>> +    {
>> +  error ("%<%s%> requires long double to be IBM 128-bit format",
>
> as a keyword long double should be quoted in the message.

Thanks, Martin, good point.  Sorry for overlooking that!

Bill
>
> Martin


Re: [PATCH] rs6000: Some builtins require IBM-128 long double format (PR103623)

2021-12-13 Thread Martin Sebor via Gcc-patches

On 12/13/21 8:55 AM, Bill Schmidt via Gcc-patches wrote:

Hi!

PR103623 shows that we ICE if __builtin_pack_longdouble or 
__builtin_unpack_longdouble
is used when long double is not defined to be the IBM-128 (double-double) 
format.
To solve this, I introduce a new built-in function attribute "ibmld" that 
enforces
this requirement.

Tested the fix on a powerpc-e300c3-linux-gnu cross.  Bootstrapped and tested on
powerpc64le-linux-gnu with no regressions.  Is this okay for trunk?


Just a minor note about the format of the new error message
below:

...

diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index d9736eaf21c..b6f0c6c4c08 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -15741,6 +15741,13 @@ rs6000_expand_new_builtin (tree exp, rtx target,
return const0_rtx;
  }
  
+  if (bif_is_ibmld (*bifaddr) && !FLOAT128_2REG_P (TFmode))

+{
+  error ("%<%s%> requires long double to be IBM 128-bit format",


as a keyword long double should be quoted in the message.

Martin


[PATCH] rs6000: Some builtins require IBM-128 long double format (PR103623)

2021-12-13 Thread Bill Schmidt via Gcc-patches
Hi!

PR103623 shows that we ICE if __builtin_pack_longdouble or 
__builtin_unpack_longdouble
is used when long double is not defined to be the IBM-128 (double-double) 
format.
To solve this, I introduce a new built-in function attribute "ibmld" that 
enforces
this requirement.

Tested the fix on a powerpc-e300c3-linux-gnu cross.  Bootstrapped and tested on
powerpc64le-linux-gnu with no regressions.  Is this okay for trunk?

Thanks!
Bill


2021-12-13  Bill Schmidt  

gcc/
PR target/103623
* config/rs6000/rs6000-builtin-new.def (__builtin_pack_longdouble): Add
ibmld attribute.
(__builtin_unpack_longdouble): Likewise.
* config/rs6000/rs6000-call.c (rs6000_expand_new_builtin): Add special
handling for ibmld attribute.
* config/rs6000/rs6000-gen-builtins.c (attrinfo): Add isibmld.
(parse_bif_attrs): Handle ibmld.
(write_decls): Likewise.
(write_bif_static_init): Likewise.
---
 gcc/config/rs6000/rs6000-builtin-new.def | 11 +++
 gcc/config/rs6000/rs6000-call.c  |  7 +++
 gcc/config/rs6000/rs6000-gen-builtins.c  | 13 +++--
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtin-new.def 
b/gcc/config/rs6000/rs6000-builtin-new.def
index 2becd96a36c..a020dbbe2fb 100644
--- a/gcc/config/rs6000/rs6000-builtin-new.def
+++ b/gcc/config/rs6000/rs6000-builtin-new.def
@@ -137,6 +137,7 @@
 ;   lxvrse   Needs special handling for load-rightmost, sign-extended
 ;   lxvrze   Needs special handling for load-rightmost, zero-extended
 ;   endian   Needs special handling for endianness
+;   ibmldRestrict usage to the case when TFmode is IBM-128
 ;
 ; Each attribute corresponds to extra processing required when
 ; the built-in is expanded.  All such special processing should
@@ -215,13 +216,8 @@
   double __builtin_mffsl ();
 MFFSL rs6000_mffsl {}
 
-; This thing really assumes long double == __ibm128, and I'm told it has
-; been used as such within libgcc.  Given that __builtin_pack_ibm128
-; exists for the same purpose, this should really not be used at all.
-; TODO: Consider adding special handling for this to warn whenever
-; long double is not __ibm128.
   const long double __builtin_pack_longdouble (double, double);
-PACK_TF packtf {}
+PACK_TF packtf {ibmld}
 
   unsigned long __builtin_ppc_mftb ();
 MFTB rs6000_mftb_di {32bit}
@@ -244,9 +240,8 @@
   const double __builtin_unpack_ibm128 (__ibm128, const int<1>);
 UNPACK_IF unpackif {}
 
-; See above comments for __builtin_pack_longdouble.
   const double __builtin_unpack_longdouble (long double, const int<1>);
-UNPACK_TF unpacktf {}
+UNPACK_TF unpacktf {ibmld}
 
 
 ; Builtins that have been around just about forever, but not quite.
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index d9736eaf21c..b6f0c6c4c08 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -15741,6 +15741,13 @@ rs6000_expand_new_builtin (tree exp, rtx target,
   return const0_rtx;
 }
 
+  if (bif_is_ibmld (*bifaddr) && !FLOAT128_2REG_P (TFmode))
+{
+  error ("%<%s%> requires long double to be IBM 128-bit format",
+bifaddr->bifname);
+  return const0_rtx;
+}
+
   if (bif_is_cpu (*bifaddr))
 return new_cpu_expand_builtin (fcode, exp, target);
 
diff --git a/gcc/config/rs6000/rs6000-gen-builtins.c 
b/gcc/config/rs6000/rs6000-gen-builtins.c
index d2e9c4ce547..ababe83895c 100644
--- a/gcc/config/rs6000/rs6000-gen-builtins.c
+++ b/gcc/config/rs6000/rs6000-gen-builtins.c
@@ -92,6 +92,7 @@ along with GCC; see the file COPYING3.  If not see
  lxvrse   Needs special handling for load-rightmost, sign-extended
  lxvrze   Needs special handling for load-rightmost, zero-extended
  endian   Needs special handling for endianness
+ ibmldRestrict usage to the case when TFmode is IBM-128
 
An example stanza might look like this:
 
@@ -390,6 +391,7 @@ struct attrinfo
   bool islxvrse;
   bool islxvrze;
   bool isendian;
+  bool isibmld;
 };
 
 /* Fields associated with a function prototype (bif or overload).  */
@@ -1435,6 +1437,8 @@ parse_bif_attrs (attrinfo *attrptr)
  attrptr->islxvrze = 1;
else if (!strcmp (attrname, "endian"))
  attrptr->isendian = 1;
+   else if (!strcmp (attrname, "ibmld"))
+ attrptr->isibmld = 1;
else
  {
diag (oldpos, "unknown attribute.\n");
@@ -1468,14 +1472,14 @@ parse_bif_attrs (attrinfo *attrptr)
"ldvec = %d, stvec = %d, reve = %d, pred = %d, htm = %d, "
"htmspr = %d, htmcr = %d, mma = %d, quad = %d, pair = %d, "
"mmaint = %d, no32bit = %d, 32bit = %d, cpu = %d, ldstmask = %d, "
-   "lxvrse = %d, lxvrze = %d, endian = %d.\n",
+   "lxvrse = %d, lxvrze = %d, endian = %d, ibmdld= %d.\n",
attrptr->isinit, attrptr->isset, attrptr->isextract,
attrptr->isnosoft, attrptr->isldvec, attrpt