[PATCH] Add Clang support for PPC cryptography builtins

[Bill Schmidt]

In http://reviews.llvm.org/D7951#131485, @hfinkel wrote:

> In http://reviews.llvm.org/D7951#131459, @wschmidt wrote:
>
> > In http://reviews.llvm.org/D7951#131441, @hfinkel wrote:
> >
> > > In http://reviews.llvm.org/D7951#131439, @nemanjai wrote:
> > >
> > > > Note to reviewers: There is currently no macro guard for the builtins that do not require Cagegory:Vector.Crypto. However, the back end will not generate code for them on older CPU's. Perhaps I should guard those with __POWER8_VECTOR__ macro. However, this would imply that -mcrypto needs to imply -mpower8-vector which is probably the correct thing to do since Category:Vector.Crypto is a subset of Category:Vector.
> > > >  I can make these changes and upload a revision if everyone agrees with this approach.
> > >
> > >
> > > Why don't we guard them all with __CRYPTO__? It seems somewhat odd to have some, but not all, of the __builtin_crypto_* available when the crypto feature is disabled.
> >
> >
> > Because this is too big of a hammer.  GCC made a mistake with this (I've proposed correcting this and will be working on fixing it in the future).  We need to treat the SHA and AES support instructions as a separate group because Vector.Crypto is an optional implementation feature in the hardware, due to export control restrictions.  POWER8 hardware with such instructions couldn't be transported legally to sensitive countries.  But the other instructions in that section of the ISA are under no such legal restrictions, and as they are part of the Vector feature, they must be implemented for OpenPOWER-compliant implementations.
>
>
> Understood. Exactly what have you proposed that GCC do?
>
> My preference would be the following: Define intrinsics for these instructions without 'crypto' in the name, and make them available predicated only on __POWER8_VECTOR__. Define aliases (using #define or another inline function) to these with 'crypto' in the name, and have these available predicated on __CRYPTO__. This way the instructions remain generally available, but we don't end up in the confusing situation where __builtin_crypto_* functions are available even when the 'crypto' feature has been disabled.


In general, I agree with you.  We have an open issue where GCC and the XL compilers have diverged in naming of the crypto builtins, and we need to get agreement on what the new names would be.  Your proposal is a good place to start.  We just haven't gotten around to cleaning that aspect up.

In the short term, I'd like to keep the same builtin names as GCC so we don't end up with 3 sets of names to reconcile, and to provide compatibility for any existing code that uses them.  The existing names will be deprecated as soon as we can get new names in place, but I think we still need them to avoid compatibility issues in the near term.

Note that the __builtin_crypto_* forms of these instructions are documented in table A.3 (Optional Built-In Functions) of the ELF V2 ABI, so that's another reason to keep them for now.

(I believe we got in this situation because the Vector.Crypto designation was added very late in the POWER8 development cycle, after the binutils and builtin work for GCC was already done.  The export restriction should have been caught much earlier, but wasn't.)


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D7951

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/