[v2] rs6000: Fix a handful of 32-bit built-in function problems

  Hi!  I previously posted [1] to correct some problems with the new builtins
support targeting 32-bit code gen.  Based on the discussion, I've made some
adjustments and would like to submit this for consideration.

We eventually agreed that the strange behavior for -m32 -mpowerpc64 for certain
HTM builtins should be removed.  All of the registers TEXASR, TEXASRU, TFHAR,
and TFIAR are now accessed using the unsigned long data type in all configurations.

Segher didn't like the change in the error message for the cmpb-3.c test case,
but I think this should be fine.  The test case just tests for the error message,
but there is also a "note" message that provides additional information.  The
diagnostics that the user sees will look like this:

cmpb-3.c:11:3: error: '__builtin_p6_cmpb' requires the '-mcpu=power6' option and either the '-m64' or '-mpowerpc64' option
cmpb-3.c:11:3: note: builtin '__builtin_cmpb' requires builtin '__builtin_p6_cmpb'

So it's clear to the user that their use of __builtin_cmpb at line 11 triggered
the error.

Bootstrapped and tested on powerpc64le-linux-gnu, and on powerpc64-linux-gnu
using -m32/-m64.  Is this okay for trunk?

Thanks!
Bill

[1] https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583905.html


2021-11-16  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	* config/rs6000/rs6000-builtin-new.def (CMPB): Flag as no32bit.
	(BPERMD): Flag as 32bit (needing special handling for 32-bit).
	(UNPACK_TD): Return unsigned long long instead of unsigned long.
	(GET_TEXASR): Return unsigned long instead of unsigned long long.
	(GET_TEXASRU): Likewise.
	(GET_TFHAR): Likewise.
	(GET_TFIAR): Likewise.
	(SET_TEXASR): Pass unsigned long instead of unsigned long long.
	(SET_TEXASRU): Likewise.
	(SET_TFHAR): Likewise.
	(SET_TFIAR): Likewise.
	(TABORTDC): Likewise.
	(TABORTDCI): Likewise.
	* config/rs6000/rs6000-call.c (rs6000_expand_new_builtin): Fix error
	handling for no32bit.  Add 32bit handling for RS6000_BIF_BPERMD.

gcc/testsuite/
	* gcc.target/powerpc/cmpb-3.c: Adjust error message.
---
 gcc/config/rs6000/rs6000-builtin-new.def  | 30 +++++++++++------------
 gcc/config/rs6000/rs6000-call.c           |  9 ++++---
 gcc/testsuite/gcc.target/powerpc/cmpb-3.c |  2 +-
 3 files changed, 22 insertions(+), 19 deletions(-)
  

Hi!

I'd like to ping this patch.  By the way, the diagnostics are improved [1] since I
sent it, so that we now inform the user that the overloaded function is implemented
by the instantiated function.

Thanks!
Bill

[1] https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585250.html

On 11/16/21 12:56 PM, Bill Schmidt wrote:
> Hi!  I previously posted [1] to correct some problems with the new builtins
> support targeting 32-bit code gen.  Based on the discussion, I've made some
> adjustments and would like to submit this for consideration.
>
> We eventually agreed that the strange behavior for -m32 -mpowerpc64 for certain
> HTM builtins should be removed.  All of the registers TEXASR, TEXASRU, TFHAR,
> and TFIAR are now accessed using the unsigned long data type in all configurations.
>
> Segher didn't like the change in the error message for the cmpb-3.c test case,
> but I think this should be fine.  The test case just tests for the error message,
> but there is also a "note" message that provides additional information.  The
> diagnostics that the user sees will look like this:
>
> cmpb-3.c:11:3: error: '__builtin_p6_cmpb' requires the '-mcpu=power6' option and either the '-m64' or '-mpowerpc64' option
> cmpb-3.c:11:3: note: builtin '__builtin_cmpb' requires builtin '__builtin_p6_cmpb'
>
> So it's clear to the user that their use of __builtin_cmpb at line 11 triggered
> the error.
>
> Bootstrapped and tested on powerpc64le-linux-gnu, and on powerpc64-linux-gnu
> using -m32/-m64.  Is this okay for trunk?
>
> Thanks!
> Bill
>
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583905.html
>
>
> 2021-11-16  Bill Schmidt  <wschmidt@linux.ibm.com>
>
> gcc/
> 	* config/rs6000/rs6000-builtin-new.def (CMPB): Flag as no32bit.
> 	(BPERMD): Flag as 32bit (needing special handling for 32-bit).
> 	(UNPACK_TD): Return unsigned long long instead of unsigned long.
> 	(GET_TEXASR): Return unsigned long instead of unsigned long long.
> 	(GET_TEXASRU): Likewise.
> 	(GET_TFHAR): Likewise.
> 	(GET_TFIAR): Likewise.
> 	(SET_TEXASR): Pass unsigned long instead of unsigned long long.
> 	(SET_TEXASRU): Likewise.
> 	(SET_TFHAR): Likewise.
> 	(SET_TFIAR): Likewise.
> 	(TABORTDC): Likewise.
> 	(TABORTDCI): Likewise.
> 	* config/rs6000/rs6000-call.c (rs6000_expand_new_builtin): Fix error
> 	handling for no32bit.  Add 32bit handling for RS6000_BIF_BPERMD.
>
> gcc/testsuite/
> 	* gcc.target/powerpc/cmpb-3.c: Adjust error message.
> ---
>  gcc/config/rs6000/rs6000-builtin-new.def  | 30 +++++++++++------------
>  gcc/config/rs6000/rs6000-call.c           |  9 ++++---
>  gcc/testsuite/gcc.target/powerpc/cmpb-3.c |  2 +-
>  3 files changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/gcc/config/rs6000/rs6000-builtin-new.def b/gcc/config/rs6000/rs6000-builtin-new.def
> index 58dfce1ca37..30556e5c7f2 100644
> --- a/gcc/config/rs6000/rs6000-builtin-new.def
> +++ b/gcc/config/rs6000/rs6000-builtin-new.def
> @@ -273,7 +273,7 @@
>  ; Power6 builtins requiring 64-bit GPRs (even with 32-bit addressing).
>  [power6-64]
>    const signed long __builtin_p6_cmpb (signed long, signed long);
> -    CMPB cmpbdi3 {}
> +    CMPB cmpbdi3 {no32bit}
>  
>  
>  ; AltiVec builtins.
> @@ -2018,7 +2018,7 @@
>      ADDG6S addg6s {}
>  
>    const signed long __builtin_bpermd (signed long, signed long);
> -    BPERMD bpermd_di {}
> +    BPERMD bpermd_di {32bit}
>  
>    const unsigned int __builtin_cbcdtd (unsigned int);
>      CBCDTD cbcdtd {}
> @@ -2971,7 +2971,7 @@
>    void __builtin_set_fpscr_drn (const int[0,7]);
>      SET_FPSCR_DRN rs6000_set_fpscr_drn {}
>  
> -  const unsigned long __builtin_unpack_dec128 (_Decimal128, const int<1>);
> +  const unsigned long long __builtin_unpack_dec128 (_Decimal128, const int<1>);
>      UNPACK_TD unpacktd {}
>  
>  
> @@ -3014,39 +3014,39 @@
>  
>  
>  [htm]
> -  unsigned long long __builtin_get_texasr ();
> +  unsigned long __builtin_get_texasr ();
>      GET_TEXASR nothing {htm,htmspr}
>  
> -  unsigned long long __builtin_get_texasru ();
> +  unsigned long __builtin_get_texasru ();
>      GET_TEXASRU nothing {htm,htmspr}
>  
> -  unsigned long long __builtin_get_tfhar ();
> +  unsigned long __builtin_get_tfhar ();
>      GET_TFHAR nothing {htm,htmspr}
>  
> -  unsigned long long __builtin_get_tfiar ();
> +  unsigned long __builtin_get_tfiar ();
>      GET_TFIAR nothing {htm,htmspr}
>  
> -  void __builtin_set_texasr (unsigned long long);
> +  void __builtin_set_texasr (unsigned long);
>      SET_TEXASR nothing {htm,htmspr}
>  
> -  void __builtin_set_texasru (unsigned long long);
> +  void __builtin_set_texasru (unsigned long);
>      SET_TEXASRU nothing {htm,htmspr}
>  
> -  void __builtin_set_tfhar (unsigned long long);
> +  void __builtin_set_tfhar (unsigned long);
>      SET_TFHAR nothing {htm,htmspr}
>  
> -  void __builtin_set_tfiar (unsigned long long);
> +  void __builtin_set_tfiar (unsigned long);
>      SET_TFIAR nothing {htm,htmspr}
>  
>    unsigned int __builtin_tabort (unsigned int);
>      TABORT tabort {htm,htmcr}
>  
> -  unsigned int __builtin_tabortdc (unsigned long long, unsigned long long, \
> -                                   unsigned long long);
> +  unsigned int __builtin_tabortdc (unsigned long, unsigned long, \
> +                                   unsigned long);
>      TABORTDC tabortdc {htm,htmcr}
>  
> -  unsigned int __builtin_tabortdci (unsigned long long, unsigned long long, \
> -                                    unsigned long long);
> +  unsigned int __builtin_tabortdci (unsigned long, unsigned long, \
> +                                    unsigned long);
>      TABORTDCI tabortdci {htm,htmcr}
>  
>    unsigned int __builtin_tabortwc (unsigned int, unsigned int, unsigned int);
> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
> index 822a9736591..85fec80c6d7 100644
> --- a/gcc/config/rs6000/rs6000-call.c
> +++ b/gcc/config/rs6000/rs6000-call.c
> @@ -15740,9 +15740,10 @@ rs6000_expand_new_builtin (tree exp, rtx target,
>      }
>  
>    if (bif_is_no32bit (*bifaddr) && TARGET_32BIT)
> -    fatal_error (input_location,
> -		 "%<%s%> is not supported in 32-bit mode",
> -		 bifaddr->bifname);
> +    {
> +      error ("%<%s%> is not supported in 32-bit mode", bifaddr->bifname);
> +      return const0_rtx;
> +    }
>  
>    if (bif_is_cpu (*bifaddr))
>      return new_cpu_expand_builtin (fcode, exp, target);
> @@ -15766,6 +15767,8 @@ rs6000_expand_new_builtin (tree exp, rtx target,
>      {
>        if (fcode == RS6000_BIF_MFTB)
>  	icode = CODE_FOR_rs6000_mftb_si;
> +      else if (fcode == RS6000_BIF_BPERMD)
> +	icode = CODE_FOR_bpermd_si;
>        else
>  	gcc_unreachable ();
>      }
> diff --git a/gcc/testsuite/gcc.target/powerpc/cmpb-3.c b/gcc/testsuite/gcc.target/powerpc/cmpb-3.c
> index de111a80144..75641bdb22c 100644
> --- a/gcc/testsuite/gcc.target/powerpc/cmpb-3.c
> +++ b/gcc/testsuite/gcc.target/powerpc/cmpb-3.c
> @@ -8,7 +8,7 @@ void abort ();
>  long long int
>  do_compare (long long int a, long long int b)
>  {
> -  return __builtin_cmpb (a, b);	/* { dg-error "'__builtin_cmpb' is not supported in this compiler configuration" } */
> +  return __builtin_cmpb (a, b);	/* { dg-error "'__builtin_p6_cmpb' requires the '-mcpu=power6' option and either the '-m64' or '-mpowerpc64' option" } */
>  }
>  
>  void expect (long long int pattern, long long int value)
  

On Tue, Nov 16, 2021 at 12:56:52PM -0600, Bill Schmidt wrote:
> Hi!  I previously posted [1] to correct some problems with the new builtins
> support targeting 32-bit code gen.  Based on the discussion, I've made some
> adjustments and would like to submit this for consideration.
> 
> We eventually agreed that the strange behavior for -m32 -mpowerpc64 for certain
> HTM builtins should be removed.  All of the registers TEXASR, TEXASRU, TFHAR,
> and TFIAR are now accessed using the unsigned long data type in all configurations.

> gcc/
> 	* config/rs6000/rs6000-builtin-new.def (CMPB): Flag as no32bit.
> 	(BPERMD): Flag as 32bit (needing special handling for 32-bit).
> 	(UNPACK_TD): Return unsigned long long instead of unsigned long.
> 	(GET_TEXASR): Return unsigned long instead of unsigned long long.
> 	(GET_TEXASRU): Likewise.
> 	(GET_TFHAR): Likewise.
> 	(GET_TFIAR): Likewise.
> 	(SET_TEXASR): Pass unsigned long instead of unsigned long long.
> 	(SET_TEXASRU): Likewise.
> 	(SET_TFHAR): Likewise.
> 	(SET_TFIAR): Likewise.
> 	(TABORTDC): Likewise.
> 	(TABORTDCI): Likewise.
> 	* config/rs6000/rs6000-call.c (rs6000_expand_new_builtin): Fix error
> 	handling for no32bit.  Add 32bit handling for RS6000_BIF_BPERMD.
> 
> gcc/testsuite/
> 	* gcc.target/powerpc/cmpb-3.c: Adjust error message.

Okay for trunk.  Thanks!


Could you put some short blurb about the changed prototype of the HTM
reg builtins in the release notes please?  Thanks x2 :-)


Segher
  

Hi!

On 12/1/21 3:08 PM, Segher Boessenkool wrote:
> On Tue, Nov 16, 2021 at 12:56:52PM -0600, Bill Schmidt wrote:
>> Hi!  I previously posted [1] to correct some problems with the new builtins
>> support targeting 32-bit code gen.  Based on the discussion, I've made some
>> adjustments and would like to submit this for consideration.
>>
>> We eventually agreed that the strange behavior for -m32 -mpowerpc64 for certain
>> HTM builtins should be removed.  All of the registers TEXASR, TEXASRU, TFHAR,
>> and TFIAR are now accessed using the unsigned long data type in all configurations.
>> gcc/
>> 	* config/rs6000/rs6000-builtin-new.def (CMPB): Flag as no32bit.
>> 	(BPERMD): Flag as 32bit (needing special handling for 32-bit).
>> 	(UNPACK_TD): Return unsigned long long instead of unsigned long.
>> 	(GET_TEXASR): Return unsigned long instead of unsigned long long.
>> 	(GET_TEXASRU): Likewise.
>> 	(GET_TFHAR): Likewise.
>> 	(GET_TFIAR): Likewise.
>> 	(SET_TEXASR): Pass unsigned long instead of unsigned long long.
>> 	(SET_TEXASRU): Likewise.
>> 	(SET_TFHAR): Likewise.
>> 	(SET_TFIAR): Likewise.
>> 	(TABORTDC): Likewise.
>> 	(TABORTDCI): Likewise.
>> 	* config/rs6000/rs6000-call.c (rs6000_expand_new_builtin): Fix error
>> 	handling for no32bit.  Add 32bit handling for RS6000_BIF_BPERMD.
>>
>> gcc/testsuite/
>> 	* gcc.target/powerpc/cmpb-3.c: Adjust error message.
> Okay for trunk.  Thanks!
>
>
> Could you put some short blurb about the changed prototype of the HTM
> reg builtins in the release notes please?  Thanks x2 :-)

Already done!

Bill

>
> Segher