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

Bill Schmidt wschmidt@linux.ibm.com
Sun Nov 14 14:17:41 GMT 2021


On 11/11/21 10:50 AM, Bill Schmidt wrote:
> On 11/11/21 7:11 AM, Segher Boessenkool wrote:
>> 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.
>
So, we have one of the more bizarre API situations here that I've ever seen.

We have three 64-bit HTM registers:  TEXASR, TFHAR, and TFIAR.  We also have the
32-bit TEXASRU, which is the upper half of TEXASR.  The documnted interfaces for
reading and modifying these registers are:

  unsigned long __builtin_get_texasr (void);
  unsigned long __builtin_get_texasru (void);
  unsigned long __builtin_get_tfhar (void);
  unsigned long __builtin_get_tfiar (void);

  void __builtin_set_texasr (unsigned long);
  void __builtin_set_texasru (unsigned long);
  void __builtin_set_tfhar (unsigned long);
  void __builtin_set_tfiar (unsigned long);

In reality, these interfaces are defined this way for pure 32-bit and pure 64-bit,
but for -m32 -mpowerpc64 we have some grotesque hackery that overrides the
expected interfaces to be:

  unsigned long long __builtin_get_texasr (void);
  unsigned long long __builtin_get_texasru (void);
  unsigned long long __builtin_get_tfhar (void);
  unsigned long long __builtin_get_tfiar (void);

  void __builtin_set_texasr (unsigned long long);
  void __builtin_set_texasru (unsigned long long);
  void __builtin_set_tfhar (unsigned long long);
  void __builtin_set_tfiar (unsigned long long);

An undocumented conditional API is a really, really bad idea, given that it
forces users of this interface for general code to #ifdef on the -m32
-mpowerpc64 condition.  Not to mention treating 32-bit registers the same as
64-bit ones, and only modifying half the register on a 32-bit system.  (Is HTM
even supported on a 32-bit system?)

It would have likely been better to have one consistent interface, using
int for TEXASRU and long long for the others, even though that requires
dealing with two registers for the 32-bit case; but that's all water under
the bridge.  We have what we have.

If I sound irritated, it's because, just for this case, I'll have to add a
bunch of extra machinery to track up to two prototypes for each builtin
function, and perform conditional initialization when it applies.  The one
good thing is that these already have a builtin attribute "htmspr" that I
can key off of to do the extra processing.

And somebody ought to fix the misleading documentation...

Thanks,
Bill



More information about the Gcc-patches mailing list