[PATCH] D105462: [X86] Add CRC32 feature.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/Basic/Targets/X86.cpp:158
 
+  // Enable CRC32 if SSE4.2 is enabled and CRC32 is not explicitly set.
+  I = Features.find("sse4.2");
----------------
Why doesn't this say "not explicitly disabled" like the others above?


================
Comment at: clang/lib/Headers/ia32intrin.h:19
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
 #define __DEFAULT_FN_ATTRS_SSE42 __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+#define __DEFAULT_FN_ATTRS_CRC32 __attribute__((__always_inline__, __nodebug__, __target__("crc32")))
----------------
Is __DEFAULT_FN_ATTRS_SSE42 dead now?


================
Comment at: clang/lib/Headers/smmintrin.h:2358
-///    operand \a __D.
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_mm_crc32_u8(unsigned int __C, unsigned char __D)
----------------
Was min vector width incorrectly being applied to these before?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105462