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

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:2218
     Group<m_ppc_Features_Group>;
-def mpower8_crypto : Flag<["-"], "mcrypto">,
-    Group<m_ppc_Features_Group>;
-def mnopower8_crypto : Flag<["-"], "mno-crypto">,
-    Group<m_ppc_Features_Group>;
+def mcrypto : Flag<["-"], "mcrypto">, Group<m_Group>,
+    HelpText<"Add use of cryptographic instructions (ARM/PowerPC only)">;
----------------
Can you move it out of ppc specific options area to more generic options location e.g. like hard-float?


================
Comment at: clang/include/clang/Driver/Options.td:2219
+def mcrypto : Flag<["-"], "mcrypto">, Group<m_Group>,
+    HelpText<"Add use of cryptographic instructions (ARM/PowerPC only)">;
+def mnocrypto : Flag<["-"], "mno-crypto">, Group<m_Group>,
----------------
ARM/AArch64/PowerPC


================
Comment at: clang/include/clang/Driver/Options.td:2221
+def mnocrypto : Flag<["-"], "mno-crypto">, Group<m_Group>,
+    HelpText<"Disallow use of cryptographic instructions (ARM/PowerPC only)">;
 def mdirect_move : Flag<["-"], "mdirect-move">,
----------------
ARM/AArch64/PowerPC


================
Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:192
+  // En/disable crypto
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto,
+                               options::OPT_mgeneral_regs_only)) {
----------------
I believe this should be merged with the code for OPT_mgeneral_regs_only otherwise  the next if statement  for mgeneral-regs-only  would force "-crypto" .

if (A->getOption().matches(OPT_mgeneral_regs_only)))
..// disable crypto, neon etc.
else if (A->getOption().matches(options::OPT_mcrypto))
// enable crypto
...

Please also add tests when mgeneral-regs-only is specified with "-mcrypto"  before/after.


================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:453
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto)) {
+    if (A->getOption().matches(options::OPT_mcrypto) && ABI != arm::FloatABI::Soft)
+      Features.push_back("+crypto");
----------------
Please add a test for interaction with soft-float ABI option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472