[PATCH] D39521: [x86 TargetInfo] Pull CPU handling for the x86 TargetInfo into a .def file.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D39521#913337, @rsmith wrote:

> Thanks, this looks like a nice cleanup. I wonder of some of the switches over `CPUKind` (setting default features and macros) could also be moved into the .def file, but let's get this landed first and then see if that looks like an improvement.


Thanks for the feedback Richard!  For the CPUKind features, I'm hoping to talk @craig.topper into exposing those somehow from llvm, and just grabbing them there :)  Additionally, he has some other ideas that we're going over on how to expose the CpuIs information from LLVM, rather than have version of the list here.

SO, this patch might get a bit of a diet sometime tomorrow.

Thanks!
-Erich



================
Comment at: lib/Basic/Targets/X86.h:103-106
+#undef PROC_FAMILY
+#undef PROC
+#undef PROC_VENDOR
+#undef IGNORE_ALIASES
----------------
rsmith wrote:
> Our convention is for the .def files to be callee-cleanup: the .def file should `#undef` all the macros it takes as input, rather than them being `#undef`'d by the `#include`r.
Ah, i missed that!  Will do.


================
Comment at: lib/Basic/Targets/X86.h:113-126
+  // \brief Returns a pair of unsigned integers that contain the __cpu_model
+  // information required to identify the specified Processor Family/Processor.
+  // A valid Processor Family will return a 1 in the first value, and the 'type'
+  // identifier for the family in the second.  A valid Processor will return a 2
+  // in the first value, and the 'subtype' identifier for the processor in the
+  // second. If the value provided is not valid for cpu_is identification, will
+  // return a 0 in the second value.
----------------
rsmith wrote:
> Use a `struct` here rather than a pair/tuple of `unsigned`.
I can do that... the only real issue is attempting to keep that generic enough that other processors can use it.  


================
Comment at: lib/CodeGen/CGCall.cpp:1893
+        FuncAttrs.addAttribute("target-cpu",
+                               getTarget().normalizeCpuName(TargetCPU));
       if (!Features.empty()) {
----------------
rsmith wrote:
> Is this part of the refactoring or a separate change?
This is a part of this refactoring.  The 'alias' system that this patch creates would end up leaking into the backend, so this normalizes back to the non-aliased versions.  


https://reviews.llvm.org/D39521