[NFC][clang] Use X86 Features declaration from X86TargetParser

[a.elovikov]

Summary

...instead of redeclaring them in clang's own X86Target.def. They were already
required to be in sync (IIUC), so no reason to maintain two identical lists.

[erichkeane]

Are these in the same order in X86TargetParser.Def? Can we make some comment in that file to make sure that we keep them in the order (see comment in original x86Target.def)?

[craig.topper]

The order in X86Parser.def is determined by how bits are allocated in to feature vector in libgcc. They're not in any priority order that I know of.

[a.elovikov]

Another version. My main goal here is to gradually move more stuff from clang/
to llvm/ so I'm open to other suggestions in doing so.

[craig.topper]

clang/lib/Basic/Targets/X86.cpp

1061

priorites -> priorities

1067

Maybe just wrap this all in #ifndef NDEBUG?

1069

array_lengthof(Priorities) - 1

[erichkeane]

clang/lib/Basic/Targets/X86.cpp

1071

If all you care about is whether they are a consecutive range, why not just use std::is_sorted?

[craig.topper]

clang/lib/Basic/Targets/X86.cpp

1071

The Priorities array isn't sorted. It's just whatever order the X86_FEATURE_COMPAT lists them.

The values need to be unique and in a contiguous range.

[erichkeane]

clang/lib/Basic/Targets/X86.cpp

1071

Then I'd suggest something like: llvm::sort, then assert *(end - 1) - *begin == std::distance(begin, end) && llvm::adjacent_find or something.

I definitely didn't get that point out of this odd for-loop and is_contained. There is perhaps at trick with std::min and std::max too. Though, it looks like this is perhaps trying to prove that the range is 0 to the the array size, right? In that case, perhaps there is something easier.

Also a nit, it is consecutive in that case.

[erichkeane]

clang/lib/Basic/Targets/X86.cpp

1071

Actually...

std::array<unsigned, array_lengthof(Priorities) -1> HelperList;
std::iota(HelperList.begin(), HelperList.end());
std::is_permutation(HelperList.begin(), HelperList.end(),  
        std::begin(Priorities), std::end(Priorities));

[a.elovikov]

Apply reviewers' suggestions. Thanks!

[a.elovikov]

clang/lib/Basic/Targets/X86.cpp

1071

I thought about std::sort + std::iota + std::equal but wasn't sure how readable that would be with the extra helper object (range version isn't available in C++14) and hoped my version would be more compact.

Since it wasn't obvious what it does I shamelessly copied your suggestion (and learnt about std::is_permutation and array_lengthof when doing it).

Thanks!

[a.elovikov]

Hi guys, do you want me to fix anything else? I think I've addressed what I could.

[erichkeane]

2 nits, give Craig a day or two to =1 as well please, particularly since he's the code-owner here.

clang/lib/Basic/Targets/X86.cpp

1062

Just a nit, I would like a better description here of the constraint we're trying to enforce. I can imagine someone in the future failing this, and not getting the context without a few sentences of how no duplicates are allowed, and it must be all numbers from 0 to the-max-number.

1072

[craig.topper]

LGTM

[a.elovikov]

Address Erich's comments

clang/include/clang/Basic/X86Target.def

 //===--- X86Target.def - X86 Feature/Processor Database ---------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the X86-specific Features and Processors, as used by
 // the X86 Targets.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef FEATURE
-#define FEATURE(ENUM)
-#endif
-
 #ifndef CPU_SPECIFIC
 #define CPU_SPECIFIC(NAME, MANGLING, FEATURES)
 #endif
 
 #ifndef CPU_SPECIFIC_ALIAS
 #define CPU_SPECIFIC_ALIAS(NEW_NAME, NAME)
 #endif
 
-// List of CPU Supports features in order.  These need to remain in the order
-// required by attribute 'target' checking.  Note that not all are supported/
-// prioritized by GCC, so synchronization with GCC's implementation may require
-// changing some existing values.
-FEATURE(FEATURE_CMOV)
-FEATURE(FEATURE_MMX)
-FEATURE(FEATURE_SSE)
-FEATURE(FEATURE_SSE2)
-FEATURE(FEATURE_SSE3)
-FEATURE(FEATURE_SSSE3)
-FEATURE(FEATURE_SSE4_A)
-FEATURE(FEATURE_SSE4_1)
-FEATURE(FEATURE_SSE4_2)
-FEATURE(FEATURE_POPCNT)
-FEATURE(FEATURE_AES)
-FEATURE(FEATURE_PCLMUL)
-FEATURE(FEATURE_AVX)
-FEATURE(FEATURE_BMI)
-FEATURE(FEATURE_FMA4)
-FEATURE(FEATURE_XOP)
-FEATURE(FEATURE_FMA)
-FEATURE(FEATURE_BMI2)
-FEATURE(FEATURE_AVX2)
-FEATURE(FEATURE_AVX512F)
-FEATURE(FEATURE_AVX512VL)
-FEATURE(FEATURE_AVX512BW)
-FEATURE(FEATURE_AVX512DQ)
-FEATURE(FEATURE_AVX512CD)
-FEATURE(FEATURE_AVX512ER)
-FEATURE(FEATURE_AVX512PF)
-FEATURE(FEATURE_AVX512VBMI)
-FEATURE(FEATURE_AVX512IFMA)
-FEATURE(FEATURE_AVX5124VNNIW)
-FEATURE(FEATURE_AVX5124FMAPS)
-FEATURE(FEATURE_AVX512VPOPCNTDQ)
-FEATURE(FEATURE_AVX512VBMI2)
-FEATURE(FEATURE_GFNI)
-FEATURE(FEATURE_VPCLMULQDQ)
-FEATURE(FEATURE_AVX512VNNI)
-FEATURE(FEATURE_AVX512BITALG)
-FEATURE(FEATURE_AVX512BF16)
-FEATURE(FEATURE_AVX512VP2INTERSECT)
-
-
 // FIXME: When commented out features are supported in LLVM, enable them here.
 CPU_SPECIFIC("generic", 'A', "")
 CPU_SPECIFIC("pentium", 'B', "")
 CPU_SPECIFIC("pentium_pro", 'C', "+cmov")
 CPU_SPECIFIC("pentium_mmx", 'D', "+mmx")
 CPU_SPECIFIC("pentium_ii", 'E', "+cmov,+mmx")
 CPU_SPECIFIC("pentium_iii", 'H', "+cmov,+mmx,+sse")
 CPU_SPECIFIC_ALIAS("pentium_iii_no_xmm_regs", "pentium_iii")

 CPU_SPECIFIC_ALIAS("mic_avx512", "knl")
 CPU_SPECIFIC("skylake", 'b', "+cmov,+mmx,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+movbe,+popcnt,+f16c,+avx,+fma,+bmi,+lzcnt,+avx2,+adx,+mpx")
 CPU_SPECIFIC( "skylake_avx512", 'a', "+cmov,+mmx,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+movbe,+popcnt,+f16c,+avx,+fma,+bmi,+lzcnt,+avx2,+avx512dq,+avx512f,+adx,+avx512cd,+avx512bw,+avx512vl,+clwb")
 CPU_SPECIFIC("cannonlake", 'e', "+cmov,+mmx,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+movbe,+popcnt,+f16c,+avx,+fma,+bmi,+lzcnt,+avx2,+avx512dq,+avx512f,+adx,+avx512ifma,+avx512cd,+avx512bw,+avx512vl,+avx512vbmi")
 CPU_SPECIFIC("knm", 'j', "+cmov,+mmx,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+movbe,+popcnt,+f16c,+avx,+fma,+bmi,+lzcnt,+avx2,+avx512f,+adx,+avx512er,+avx512pf,+avx512cd,+avx5124fmaps,+avx5124vnniw,+avx512vpopcntdq")
 
 #undef CPU_SPECIFIC_ALIAS
 #undef CPU_SPECIFIC
-#undef PROC_64_BIT
-#undef PROC_32_BIT
-#undef FEATURE

clang/lib/Basic/Targets/X86.cpp

 
 // We can't use a generic validation scheme for the features accepted here
 // versus subtarget features accepted in the target attribute because the
 // bitfield structure that's initialized in the runtime only supports the
 // below currently rather than the full range of subtarget features. (See
 // X86TargetInfo::hasFeature for a somewhat comprehensive list).
 bool X86TargetInfo::validateCpuSupports(StringRef FeatureStr) const {
   return llvm::StringSwitch<bool>(FeatureStr)
-#define X86_FEATURE_COMPAT(ENUM, STR) .Case(STR, true)
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY) .Case(STR, true)
 #include "llvm/Support/X86TargetParser.def"
       .Default(false);
 }
 
 static llvm::X86::ProcessorFeatures getFeature(StringRef Name) {
   return llvm::StringSwitch<llvm::X86::ProcessorFeatures>(Name)
-#define X86_FEATURE_COMPAT(ENUM, STR) .Case(STR, llvm::X86::FEATURE_##ENUM)
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)                                \
+  .Case(STR, llvm::X86::FEATURE_##ENUM)
+
 #include "llvm/Support/X86TargetParser.def"
       ;
   // Note, this function should only be used after ensuring the value is
   // correct, so it asserts if the value is out of range.
 }
 
 static unsigned getFeaturePriority(llvm::X86::ProcessorFeatures Feat) {
-  enum class FeatPriority {
-#define FEATURE(FEAT) FEAT,
-#include "clang/Basic/X86Target.def"
+  // Check that priorites are set properly in the .def file.
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY) PRIORITY,
+  unsigned Priorities[] = {
+#include "llvm/Support/X86TargetParser.def"
+      std::numeric_limits<unsigned>::max() // Need to consume last comma.
   };
+  (void)Priorities;
+  for (unsigned Priority = 0;
+       Priority < sizeof(Priorities) / sizeof(unsigned) - 1; ++Priority) {
+    assert(llvm::is_contained(Priorities, Priority) &&
+           "Priorites don't form consequtive range!");
+  }
+
   switch (Feat) {
-#define FEATURE(FEAT)                                                          \
-  case llvm::X86::FEAT:                                                        \
-    return static_cast<unsigned>(FeatPriority::FEAT);
-#include "clang/Basic/X86Target.def"
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)                                \
+  case llvm::X86::FEATURE_##ENUM:                                              \
+    return PRIORITY;
+#include "llvm/Support/X86TargetParser.def"
   default:
     llvm_unreachable("No Feature Priority for non-CPUSupports Features");
   }
 }
 
 unsigned X86TargetInfo::multiVersionSortPriority(StringRef Name) const {
   // Valid CPUs have a 'key feature' that compares just better than its key
   // feature.

clang/lib/CodeGen/CGBuiltin.cpp

 
 uint64_t
 CodeGenFunction::GetX86CpuSupportsMask(ArrayRef<StringRef> FeatureStrs) {
   // Processor features and mapping to processor feature value.
   uint64_t FeaturesMask = 0;
   for (const StringRef &FeatureStr : FeatureStrs) {
     unsigned Feature =
         StringSwitch<unsigned>(FeatureStr)
-#define X86_FEATURE_COMPAT(ENUM, STR) .Case(STR, llvm::X86::FEATURE_##ENUM)
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)                                \
+  .Case(STR, llvm::X86::FEATURE_##ENUM)
 #include "llvm/Support/X86TargetParser.def"
         ;
     FeaturesMask |= (1ULL << Feature);
   }
   return FeaturesMask;
 }
 
 Value *CodeGenFunction::EmitX86CpuSupports(ArrayRef<StringRef> FeatureStrs) {

llvm/include/llvm/Support/X86TargetParser.def

 X86_CPU_SUBTYPE(INTEL_COREI7_TIGERLAKE,      "tigerlake")
 X86_CPU_SUBTYPE(INTEL_COREI7_COOPERLAKE,     "cooperlake")
 X86_CPU_SUBTYPE(INTEL_COREI7_SAPPHIRERAPIDS, "sapphirerapids")
 X86_CPU_SUBTYPE(INTEL_COREI7_ALDERLAKE,      "alderlake")
 X86_CPU_SUBTYPE(AMDFAM19H_ZNVER3,            "znver3")
 X86_CPU_SUBTYPE(INTEL_COREI7_ROCKETLAKE,     "rocketlake")
 #undef X86_CPU_SUBTYPE
 
-
-// This macro is used for cpu types present in compiler-rt/libgcc.
+// This macro is used for cpu types present in compiler-rt/libgcc. The third
+// parameter PRIORITY is as required by the attribute 'target' checking. Note
+// that not all are supported/prioritized by GCC, so synchronization with GCC's
+// implementation may require changing some existing values.
+//
+// We cannot just re-sort the list though because its order is dictated by the
+// order of bits in CodeGenFunction::GetX86CpuSupportsMask.
 #ifndef X86_FEATURE_COMPAT
-#define X86_FEATURE_COMPAT(ENUM, STR) X86_FEATURE(ENUM, STR)
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY) X86_FEATURE(ENUM, STR)
 #endif
 
 #ifndef X86_FEATURE
 #define X86_FEATURE(ENUM, STR)
 #endif
 
-X86_FEATURE_COMPAT(CMOV,            "cmov")
-X86_FEATURE_COMPAT(MMX,             "mmx")
-X86_FEATURE_COMPAT(POPCNT,          "popcnt")
-X86_FEATURE_COMPAT(SSE,             "sse")
-X86_FEATURE_COMPAT(SSE2,            "sse2")
-X86_FEATURE_COMPAT(SSE3,            "sse3")
-X86_FEATURE_COMPAT(SSSE3,           "ssse3")
-X86_FEATURE_COMPAT(SSE4_1,          "sse4.1")
-X86_FEATURE_COMPAT(SSE4_2,          "sse4.2")
-X86_FEATURE_COMPAT(AVX,             "avx")
-X86_FEATURE_COMPAT(AVX2,            "avx2")
-X86_FEATURE_COMPAT(SSE4_A,          "sse4a")
-X86_FEATURE_COMPAT(FMA4,            "fma4")
-X86_FEATURE_COMPAT(XOP,             "xop")
-X86_FEATURE_COMPAT(FMA,             "fma")
-X86_FEATURE_COMPAT(AVX512F,         "avx512f")
-X86_FEATURE_COMPAT(BMI,             "bmi")
-X86_FEATURE_COMPAT(BMI2,            "bmi2")
-X86_FEATURE_COMPAT(AES,             "aes")
-X86_FEATURE_COMPAT(PCLMUL,          "pclmul")
-X86_FEATURE_COMPAT(AVX512VL,        "avx512vl")
-X86_FEATURE_COMPAT(AVX512BW,        "avx512bw")
-X86_FEATURE_COMPAT(AVX512DQ,        "avx512dq")
-X86_FEATURE_COMPAT(AVX512CD,        "avx512cd")
-X86_FEATURE_COMPAT(AVX512ER,        "avx512er")
-X86_FEATURE_COMPAT(AVX512PF,        "avx512pf")
-X86_FEATURE_COMPAT(AVX512VBMI,      "avx512vbmi")
-X86_FEATURE_COMPAT(AVX512IFMA,      "avx512ifma")
-X86_FEATURE_COMPAT(AVX5124VNNIW,    "avx5124vnniw")
-X86_FEATURE_COMPAT(AVX5124FMAPS,    "avx5124fmaps")
-X86_FEATURE_COMPAT(AVX512VPOPCNTDQ, "avx512vpopcntdq")
-X86_FEATURE_COMPAT(AVX512VBMI2,     "avx512vbmi2")
-X86_FEATURE_COMPAT(GFNI,            "gfni")
-X86_FEATURE_COMPAT(VPCLMULQDQ,      "vpclmulqdq")
-X86_FEATURE_COMPAT(AVX512VNNI,      "avx512vnni")
-X86_FEATURE_COMPAT(AVX512BITALG,    "avx512bitalg")
-X86_FEATURE_COMPAT(AVX512BF16,      "avx512bf16")
-X86_FEATURE_COMPAT(AVX512VP2INTERSECT, "avx512vp2intersect")
+X86_FEATURE_COMPAT(CMOV,            "cmov",                  0)
+X86_FEATURE_COMPAT(MMX,             "mmx",                   1)
+X86_FEATURE_COMPAT(POPCNT,          "popcnt",                9)
+X86_FEATURE_COMPAT(SSE,             "sse",                   2)
+X86_FEATURE_COMPAT(SSE2,            "sse2",                  3)
+X86_FEATURE_COMPAT(SSE3,            "sse3",                  4)
+X86_FEATURE_COMPAT(SSSE3,           "ssse3",                 5)
+X86_FEATURE_COMPAT(SSE4_1,          "sse4.1",                7)
+X86_FEATURE_COMPAT(SSE4_2,          "sse4.2",                8)
+X86_FEATURE_COMPAT(AVX,             "avx",                   12)
+X86_FEATURE_COMPAT(AVX2,            "avx2",                  18)
+X86_FEATURE_COMPAT(SSE4_A,          "sse4a",                 6)
+X86_FEATURE_COMPAT(FMA4,            "fma4",                  14)
+X86_FEATURE_COMPAT(XOP,             "xop",                   15)
+X86_FEATURE_COMPAT(FMA,             "fma",                   16)
+X86_FEATURE_COMPAT(AVX512F,         "avx512f",               19)
+X86_FEATURE_COMPAT(BMI,             "bmi",                   13)
+X86_FEATURE_COMPAT(BMI2,            "bmi2",                  17)
+X86_FEATURE_COMPAT(AES,             "aes",                   10)
+X86_FEATURE_COMPAT(PCLMUL,          "pclmul",                11)
+X86_FEATURE_COMPAT(AVX512VL,        "avx512vl",              20)
+X86_FEATURE_COMPAT(AVX512BW,        "avx512bw",              21)
+X86_FEATURE_COMPAT(AVX512DQ,        "avx512dq",              22)
+X86_FEATURE_COMPAT(AVX512CD,        "avx512cd",              23)
+X86_FEATURE_COMPAT(AVX512ER,        "avx512er",              24)
+X86_FEATURE_COMPAT(AVX512PF,        "avx512pf",              25)
+X86_FEATURE_COMPAT(AVX512VBMI,      "avx512vbmi",            26)
+X86_FEATURE_COMPAT(AVX512IFMA,      "avx512ifma",            27)
+X86_FEATURE_COMPAT(AVX5124VNNIW,    "avx5124vnniw",          28)
+X86_FEATURE_COMPAT(AVX5124FMAPS,    "avx5124fmaps",          29)
+X86_FEATURE_COMPAT(AVX512VPOPCNTDQ, "avx512vpopcntdq",       30)
+X86_FEATURE_COMPAT(AVX512VBMI2,     "avx512vbmi2",           31)
+X86_FEATURE_COMPAT(GFNI,            "gfni",                  32)
+X86_FEATURE_COMPAT(VPCLMULQDQ,      "vpclmulqdq",            33)
+X86_FEATURE_COMPAT(AVX512VNNI,      "avx512vnni",            34)
+X86_FEATURE_COMPAT(AVX512BITALG,    "avx512bitalg",          35)
+X86_FEATURE_COMPAT(AVX512BF16,      "avx512bf16",            36)
+X86_FEATURE_COMPAT(AVX512VP2INTERSECT, "avx512vp2intersect", 37)
 // Features below here are not in libgcc/compiler-rt.
 X86_FEATURE       (3DNOW,           "3dnow")
 X86_FEATURE       (3DNOWA,          "3dnowa")
 X86_FEATURE       (64BIT,           "64bit")
 X86_FEATURE       (ADX,             "adx")
 X86_FEATURE       (AMX_BF16,        "amx-bf16")
 X86_FEATURE       (AMX_INT8,        "amx-int8")
 X86_FEATURE       (AMX_TILE,        "amx-tile")