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

[Segher Boessenkool <segher@kernel.crashing.org>]

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?

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

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