[PATCH] D45984: [X86] directstore and movdir64b intrinsics

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: docs/ClangCommandLineReference.rst:2465
 
+.. option:: -mmovdiri, -mno-movdiri
+
----------------
Please alphabetize this correctly.


================
Comment at: include/clang/Basic/BuiltinsX86.def:1896
+TARGET_BUILTIN(__builtin_ia32_directstore_u32, "vUi*Ui", "", "movdiri")
+TARGET_BUILTIN(__builtin_ia32_directstore_u64, "vUWi*UWi", "", "movdiri")
+TARGET_BUILTIN(__builtin_ia32_movdir64b, "vv*vC*", "", "movdir64b")
----------------
The u64 versions needs to go in BuiltinsX86_64.def. That will provide an extra safety check to prevent anyone from using the builtin without the intrin.h header on 32-bit hosts.


================
Comment at: include/clang/Driver/Options.td:2634
 def mno_cx16 : Flag<["-"], "mno-cx16">, Group<m_x86_Features_Group>;
+def mmovdiri : Flag<["-"], "mmovdiri">, Group<m_x86_Features_Group>;
+def mno_movdiri : Flag<["-"], "mno-movdiri">, Group<m_x86_Features_Group>;
----------------
Alphabetize


================
Comment at: lib/Basic/Targets/X86.cpp:1304
       .Case("cx16", true)
+      .Case("movdiri", true)
+      .Case("movdir64b", true)
----------------
Alphabetize


================
Comment at: lib/Basic/Targets/X86.cpp:1379
       .Case("cx16", HasCX16)
+      .Case("movdiri", HasMOVDIRI)
+      .Case("movdir64b", HasMOVDIR64B)
----------------
Alphabetize


================
Comment at: lib/Headers/movdirintrin.h:30
+
+#ifdef __MOVDIRI__
+
----------------
We aren't supposed to be checking the preprocessor defines in headers. We need to be able to put a target attribute on each function and control feature enabling at a function level.


================
Comment at: lib/Headers/movdirintrin.h:35
+__attribute__((__always_inline__, __nodebug__,  __target__("movdiri")))
+_directstoreu_u32 (void *__DST, __UINT32_TYPE__  __VALUE)
+{
----------------
Use 'unsigned int' instead of __UINT32_TYPE__


================
Comment at: lib/Headers/movdirintrin.h:35
+__attribute__((__always_inline__, __nodebug__,  __target__("movdiri")))
+_directstoreu_u32 (void *__DST, __UINT32_TYPE__  __VALUE)
+{
----------------
craig.topper wrote:
> Use 'unsigned int' instead of __UINT32_TYPE__
Lower case variable names


================
Comment at: lib/Headers/movdirintrin.h:45
+__attribute__((__always_inline__, __nodebug__,  __target__("movdiri")))
+_directstoreu_u64 (void *__DST, __UINT64_TYPE__ __VALUE)
+{
----------------
Use unsigned long long


Repository:
  rC Clang

https://reviews.llvm.org/D45984