[PATCH] rs6000: Fix a handful of 32-bit built-in function problems in the new support

Segher Boessenkool segher@kernel.crashing.org
Wed Nov 10 08:33:39 GMT 2021


On Tue, Nov 09, 2021 at 03:46:54PM -0600, Bill Schmidt wrote:
> Hi!  Some time ago I realized I hadn't tested the new builtin support against 32-bit
> big-endian in quite a while.  When I did, I found a handful of errors that needed
> correcting.
>  - One builtin needs to be disabled for 32-bit.
>  - One builtin needs to be restricted to 32-bit only.
>  - One builtin used unsigned long when it needed unsigned long long.
>  - Six builtins used unsigned long long when they needed unsigned long.
>  - One test case needed its expected error message adjusted.
> Otherwise things were fine.

> Bootstrapped and tested on powerpc64le-linux-gnu and powerpc64-linux-gnu with no
> regressions.

{-m32,-m64} for the latter, right?

> 	* config/rs6000/rs6000-builtin-new.def (CMPB): Flag as no32bit.
> 	(BPERMD): Flag as 32bit.
> 	(UNPACK_TD): Return unsigned long long instead of unsigned long.
> 	(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.


>    const signed long __builtin_bpermd (signed long, signed long);
> -    BPERMD bpermd_di {}
> +    BPERMD bpermd_di {32bit}

That is not what the old code does?

    case POWER7_BUILTIN_BPERMD:
      return rs6000_expand_binop_builtin (((TARGET_64BIT)
                                           ? CODE_FOR_bpermd_di
                                           : CODE_FOR_bpermd_si), exp, target);

> -  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}

This does not seem to be what the exiting code does, either?  Try with
-m32 -mpowerpc64 (it extends to 64 bit there, so the builtin does not
have long int as parameter, it has long long int).

> @@ -15758,6 +15759,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 ();
>      }

But you disabled it for 32 bit now, so huh.

> --- 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' is not supported in 32-bit mode" } */
>  }

The original spelling is the correct one?


Segher


More information about the Gcc-patches mailing list