[PATCH] TargetParser prototype - NFC (ARM only)

Renato Golin renato.golin at linaro.org
Thu May 7 09:22:42 PDT 2015


In http://reviews.llvm.org/D9435#168002, @rnk wrote:

> I guess Support is an OK place to park this particular pile of strings, enums, and tables. The target feature CPU and feature parsing is very similar in nature to triple parsing.


Yes. The idea is to have triple parsing joined with this library, and possible ARM attributes, too, into one big TargetDescription library, which will probably reside in TargetDescription.h/cpp files.

> If this reaches the logical conclusion, we'll have a file per in-tree LLVM backend, or 13 files. At that point, we might wish to make a new library.


I think I should rename it now to TargetParser, so that it's clear that this is not just ARM. Or I could call it TargetDescription.cpp/h and leave just the parser for now, then refactor it later.

cheers,
--renato


================
Comment at: include/llvm/Support/ARMTargetParser.h:23
@@ +22,3 @@
+  // FPU names.
+  // FIXME: TableGen this.
+  enum FPUKind {
----------------
rnk wrote:
> Actually, I'd much rather use .def files if possible. TableGen slows down builds and is hard to understand. Sure, pre-processor macros are hard to reason about, but they're *still* easier to understand than tablegen.
> 
> Also, so long as this library is in Support, and I think it will be here for a while, this FIXME is unactionable because Support cannot depend on TableGen.
> 
> I think we can solve any inflexibility of .def files with a separate plain C++ source synonym table, like you currently have.
The reason for using TableGen is that this information is already there. We just need to extract it with a new back-end to just output a small number of tables, instead of duplicating it here and getting it out of sync, again.

We discussed this earlier, and the consensus was that having a common TableGen run for the basic target information independent of which back-ends are compiled. Again, because that information is *already* there, and new hardware information is added in TableGen files, so these tables will be updated automatically.

With .def files, we'd probably have to populate them via TableGen to get up-to-date information anyway. Doesn't make much difference. I personally would rather have a larger header than multiple .def files.

================
Comment at: lib/Support/ARMTargetParser.cpp:104-105
@@ +103,4 @@
+const char *
+ARMTargetParser::
+getFPUName(unsigned ID) {
+  if (ID >= ARM::LAST_FPU)
----------------
rnk wrote:
> This style of separating the class name and the method name appears in LLVM, but it's really uncommon and I don't want to encourage it. Do you mind not using it?
sure! That was a side effect of refactoring. I also dislike it. :)

http://reviews.llvm.org/D9435

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list