In http://reviews.llvm.org/D7951#131497, @wschmidt wrote:
> 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.) Alright. So, for now, add a __POWER8_VECTOR__ guard around the non-crypto __builtin_crypto_* functions. Please also add a comment in the file noting the naming inconsistency, and stating that newer names are under development. REPOSITORY rL LLVM http://reviews.llvm.org/D7951 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
