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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
