[PATCH] D43817: [x86] wbnoinvd intrinsic

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: docs/ClangCommandLineReference.rst:2359
 
+.. option:: -mwbnoinvd, -mno-wbnoinvd
+
----------------
Did you manually add these? This file is normally generated by a tool and should be in alphabetical order.


================
Comment at: lib/Basic/Targets/X86.cpp:136
     // TODO: Add icelake features here.
+    setFeatureEnabledImpl(Features, "wbnoinvd", true);
     LLVM_FALLTHROUGH;
----------------
Is this based on an old repo? Icelake features have been coded here since late December. But as I said in the llvm patch we probably can't enable this on all icelakes.


================
Comment at: lib/Headers/ia32intrin.h:79
 
+#define _wbinvd() __builtin_ia32_wbinvd()
+
----------------
Can you separate wbinvd out of this patch? This has some Microsoft compatibility issues that need to be carefully checked. We seem to already have __wbinvd() defined in intrin.h but I'm not sure it does anything.


================
Comment at: lib/Headers/wbnoinvdintrin.h:31
+
+#define _wbnoinvd __builtin_ia32_wbnoinvd
+
----------------
Use an inline function, not a macro.


Repository:
  rC Clang

https://reviews.llvm.org/D43817