[PATCH] D81439: [X86] Move CPUKind enum from clang to llvm/lib/Support. NFCI

[Erich Keane via Phabricator via llvm-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Basic/Targets/X86.h:132
 protected:
-  /// Enumeration of all of the X86 CPUs supported by Clang.
-  ///
-  /// Each enumeration represents a particular CPU supported by Clang. These
-  /// loosely correspond to the options passed to '-march' or '-mtune' flags.
-  enum CPUKind {
-    CK_Generic,
-#define PROC(ENUM, STRING, IS64BIT) CK_##ENUM,
-#include "clang/Basic/X86Target.def"
-  } CPU = CK_Generic;
-
-  bool checkCPUKind(CPUKind Kind) const;
-
-  CPUKind getCPUKind(StringRef CPU) const;
+  llvm::X86::CPUKind CPU = llvm::X86::CK_None;
 
----------------
craig.topper wrote:
> erichkeane wrote:
> > This name Generic has nothing to do with the CPU-Specific 'generic' spelling, right?
> The backend has a "generic" but the frontend doesn't accept it. That's another reason I renamed the enum name.
{meme, src=brilliant}




================
Comment at: llvm/include/llvm/Support/X86TargetParser.h:29
+
+CPUKind parseArchX86(StringRef CPU, bool ArchIs32Bit = true);
+void fillValidCPUArchList(SmallVectorImpl<StringRef> &Values,
----------------
craig.topper wrote:
> erichkeane wrote:
> > Thats an odd default... Does that mean "include 32 bit", or "just 32 bit"?  And do those need to be differentiated?
> It means include all CPUs. false removes the CPUs that don't support 64 bit. A couple of the callers of getCPUKind didn't call the checkCPUKind so the default matches the behavior of not checking. For the other places I just turned the getArch() == Triple::x86 directly into this flag.
> 
> Currently we only need to remove CPUs that aren't capable of 64 bit.
> 
> Maybe I should reverse the polarity to Require64Bit or something?
Something like "Include64bit"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81439