[PATCH] D65840: [X86] Support -march=tigerlake

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/Basic/Targets/X86.cpp:167
   case CK_IcelakeServer:
-    setFeatureEnabledImpl(Features, "pconfig", true);
-    setFeatureEnabledImpl(Features, "wbnoinvd", true);
+    if (Kind != CK_Tigerlake) {
+      setFeatureEnabledImpl(Features, "pconfig", true);
----------------
xiangzhangllvm wrote:
> RKSimon wrote:
> > Are we ok with introducing this kind of thing again or should we use goto?
> Sorry, not quite understand, How to  introducing it again?
We had a bunch of ifs like this in the switch before and turned into a maintenance issue because multiple ifs needed to be updated everytime someone added something to the stop of the switch. They were removed in  https://reviews.llvm.org/D63018


================
Comment at: clang/test/Preprocessor/predefined-arch-macros.c:1535
+// CHECK_TGL_M32: #define __AVX512VNNI__ 1
+// CHECK_TGL_M32: #define __AVX512VPOPCNTDQ__ 1
+// CHECK_TGL_M32: #define __AVX__ 1
----------------
Missing a check for AVX512VP2INTERSECT


================
Comment at: clang/test/Preprocessor/predefined-arch-macros.c:1569
+// CHECK_TGL_M32: #define __VPCLMULQDQ__ 1
+// CHECK_TGL_M32-NOT: #define __WBNOINVD__ 1
+// CHECK_TGL_M32: #define __XSAVEC__ 1
----------------
xiangzhangllvm wrote:
> RKSimon wrote:
> > You test for no-wbnoinvd - do we need to do the same for pconfig?
> I am not much sure about the -wbnoinvd, The tigerlake  succeed to icelake, So I write the check follow the icelake.
I think icelake ended up like that because we had a bug once due to the ifs in the switch. Go ahead and add a NOT for PCONFIG


================
Comment at: llvm/lib/Support/Host.cpp:749
     default: // Unknown family 6 CPU, try to guess.
+      // TODO detect tigerlake host
+
----------------
RKSimon wrote:
> Check for tigerlake with avx512vp2intersect cpuid bit? @craig.topper any suggestions?
Sounds good to me.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65840