[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

Wed Apr 10 07:55:37 PDT 2019

peter.smith added a comment.

In D60472#1461336 <https://reviews.llvm.org/D60472#1461336>, @manojgupta wrote:

> The motivation for this change is to make "crypto" setting an additive option e.g. like "-mavx" used in many media packages.  Some packages in Chrome want to enable crypto conditionally for a few files to allow crypto feature to be used based on runtime cpu detection. They set "-march=armv8+crypto" flag but it gets overridden by the global "-march=armv8a" flag set by the build system in Chrome OS because the target cpu does not support crypto causing compile-time errors. 
>  Ability to specify "-mcrypto"  standalone makes it an additive option and ensures that it it is not lost. i.e. this will help in decoupling  "-mcrypto" from "-march" so that they could be set independently. The current additive alternate is  '-Xclang -target-feature -Xclang "+crypto" ' which is ugly.

Is that not a limitation of the build system? I'd expect a package to be able to locally override a global default rather than vice-versa. Although crypto might be the area of concern here and there may be a conveniently named option for PPC, where does this stop? Crypto is not the only architectural extension, for example see https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To maintain a consistent interface we'd need command line options for all the extensions. May I encourage you to reply to the RFC on command line options that I mentioned earlier if it doesn't work for you? I think the extensions need to be considered as a whole and not just individually.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60472/new/

https://reviews.llvm.org/D60472