[PATCH] D93789: Add accessors for MCSubtargetInfo CPU and Feature tables

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 07:48:52 PST 2021


fhahn added a comment.

In D93789#2505267 <https://reviews.llvm.org/D93789#2505267>, @nikic wrote:

> In D93789#2503709 <https://reviews.llvm.org/D93789#2503709>, @craig.topper wrote:
>
>> I’m not against this, but do want to mention the help text is incorrect for x86-64. It lists CPUs that don’t support 64-bit. Using one of those CPUs will trigger a fatal error. A better list is fillValidCPUList from X86TargetParser.cpp in lib/Support. It takes a mode argument. That’s the interface clang uses.
>
> The fillValidCPUArchList() interface looks pretty annoying in that it's non-virtual. One needs to call different helper functions for each target.

The current situation with `TargetParser` is indeed a bit unfortunate/half baked/unfinished. But I think it wold be preferable to improve the ergonomics of `fillValidCPUArchList`, rather than introducing a separate interface that duplicates the functionality and sometimes provides incorrect results. IMO there should be one interface used by frontends to obtain the information about supported CPUs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93789



More information about the llvm-commits mailing list