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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

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.



================
Comment at: lib/Basic/Targets/X86.h:103-106
+#undef PROC_FAMILY
+#undef PROC
+#undef PROC_VENDOR
+#undef IGNORE_ALIASES
----------------
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.


================
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.
----------------
Use a `struct` here rather than a pair/tuple of `unsigned`.


================
Comment at: lib/CodeGen/CGCall.cpp:1893
+        FuncAttrs.addAttribute("target-cpu",
+                               getTarget().normalizeCpuName(TargetCPU));
       if (!Features.empty()) {
----------------
Is this part of the refactoring or a separate change?


https://reviews.llvm.org/D39521