[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

1060–1062

priorites -> priorities

1064

Maybe just wrap this all in #ifndef NDEBUG?

1066

array_lengthof(Priorities) - 1

[erichkeane]

clang/lib/Basic/Targets/X86.cpp

1068

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

1068

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

1068

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

1068

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

1068

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

1061

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.

1069

[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( "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

 #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"
+#define X86_FEATURE_COMPAT(ENUM, STR) ENUM,
+#include "llvm/Support/X86TargetParser.def"
+    MAX
   };
-  switch (Feat) {
-#define FEATURE(FEAT)                                                          \
-  case llvm::X86::FEAT:                                                        \
-    return static_cast<unsigned>(FeatPriority::FEAT);
-#include "clang/Basic/X86Target.def"
-  default:
-    llvm_unreachable("No Feature Priority for non-CPUSupports Features");
-  }
+  assert(Feat < static_cast<unsigned>(FeatPriority::MAX) &&
+         "No Feature Priority for non-CPUSupports Features");
+  return static_cast<unsigned>(Feat);
 }
 
 unsigned X86TargetInfo::multiVersionSortPriority(StringRef Name) const {
   // Valid CPUs have a 'key feature' that compares just better than its key
   // feature.
   using namespace llvm::X86;
   CPUKind Kind = parseArchX86(Name);
   if (Kind != CK_None) {
     ProcessorFeatures KeyFeature = getKeyFeature(Kind);
     return (getFeaturePriority(KeyFeature) << 1) + 1;
   }