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

[Bill Schmidt]

Hi!

On 11/11/21 7:11 AM, Segher Boessenkool wrote:
> On Wed, Nov 10, 2021 at 03:28:18PM -0600, Bill Schmidt wrote:
>> On 11/10/21 2:33 AM, Segher Boessenkool wrote:
>>> On Tue, Nov 09, 2021 at 03:46:54PM -0600, Bill Schmidt wrote:
>>>> 	* config/rs6000/rs6000-builtin-new.def (CMPB): Flag as no32bit.
>>>> 	(BPERMD): Flag as 32bit.
> So, change this to something like "flag this as needing special handling
> on 32 bit" or something?

Sure.
>
>>>> -  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).
>> This uses a tfiar_t, which is a typedef for uintptr_t, so long int is appropriate.
>> This is necessary to make the HTM tests pass on 32-bit powerpc64.
> void f(long x) { __builtin_set_texasr(x); }
>
> built with -m32 -mpowerpc64 gives (in the expand dump):
>
> void f (long int x)
> {
>   long long unsigned int _1;
>
> ;;   basic block 2, loop depth 0
> ;;    pred:       ENTRY
>   _1 = (long long unsigned int) x_2(D);
>   __builtin_set_texasr (_1); [tail call]
>   return;
> ;;    succ:       EXIT
>
> }
>
> The builtins have a "long long" argument in the existing code, in this
> configuration.  And this is not the same as "long" here.

Hm, strange.  I'll have to go back and revisit this.  Something subtle going on.

Thanks,
Bill

>
>>>> --- 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?
>> This is something I have on my to-do list for the future, to see whether I
>> can improve it.  The overloaded function __builtin_cmpb gets translated to
>> the underlying non-overloaded builtin __builtin_p6_cmpb, and that's the only
>> name that's still around by the time we get to the error processing.  I want
>> to see whether I can add some infrastructure to recover the overloaded
>> function name in such cases.  Is it okay to defer this for now?
> It is fine to defer it.  It is not fine to change the testcase like
> this.  The user did not write __builtin_p6_cmpb (which is not even
> documented btw), so the compiler should not talk about that.  It is
> fine to leave the test failing for now.
>
>
> Segher