[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Richard Smith - zygoloid via Phabricator via llvm-commits]

rsmith added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:2004-2007
+def mspeculative_load_hardening : Flag<["-"], "mspeculative-load-hardening">,
+  Group<m_Group>, Flags<[CoreOption,CC1Option]>;
+def mno_speculative_load_hardening : Flag<["-"], "mno-speculative-load-hardening">,
+  Group<m_Group>, Flags<[CoreOption,CC1Option]>;
----------------
We usually only provide `-cc1` flags to switch into the non-default state.


================
Comment at: clang/include/clang/Frontend/CodeGenOptions.def:214
 CODEGENOPT(SoftFloat         , 1, 0) ///< -soft-float.
+CODEGENOPT(SpeculativeLoadHardening, 1, 0) ///< Enable speculative_load_hardening.
 CODEGENOPT(FineGrainedBitfieldAccesses, 1, 0) ///< Enable fine-grained bitfield accesses.
----------------
Spaces rather than underscores here.


================
Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:169-170
                           options::OPT_mno_retpoline_external_thunk, false)) {
     // FIXME: Add a warning about failing to specify `-mretpoline` and
     // eventually switch to an error here.
     Features.push_back("+retpoline-indirect-calls");
----------------
`-mspeculative-load-hardening -mretpoline-external-thunk` does not enable `+retpoline-indirect-branches` (but `-mretpoline-external-thunk` by itself does). Is that intentional?


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:1169
+  case Attribute::SpeculativeLoadHardening:
+    return 1ULL << 60;
   case Attribute::Dereferenceable:
----------------
rnk wrote:
> These appear to repeat LLVMBitcodes.h, unless I am mistaken. Weird. Anyway, fixing that is out of scope.
The values "above the fold" in Phabricator aren't all the same as the enum value plus one. *shrug*


Repository:
  rL LLVM

https://reviews.llvm.org/D51157