TargetParser - Always build all table-gen files?

Hi Folks,

I'm back looking at the target parser that we discussed last year, and
I have some questions.

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-August/038699.html

There is no way I can create a target-independent parser in lib/Target
without re-encoding all the information about the targets that we
already have in the table-gen files on all targets' directories. Doing
that would be folly.

But building all targets by default goes against the current thinking,
which I quite welcome and think we should not break it. We need to
find a common ground.

My idea is to build all table-gen files on all targets, but not build
all targets' files and not include them in the final libraries /
binaries. Since the table-gen'ed files don't end up there by
themselves, only what we use from them (in the target parser) will be
included in the final binaries.

Does that sound like a good idea? If so, how would one go about doing
that? I'm guessing it's a change solely in CMake files, and could be
done independently of the target parser implementation, right?

cheers,
--renato

Attached is a patch that kind of does that, but not completely. CMakes
finishes ok, but make can't find the td files.

Basically, I moved the tablegen() registration process to a separate
file (CMakeTblGen.txt), which is included from CMakeLists.txt in each
target directory and also in the root lib/Target/CMakeLists.txt, so
that I can NOT include all targets while including ALL table-gen files
that weren't included by the targets_to_build.

The problem is that, when I include it from lib/Target/CMakeLists.txt,
the tablegen(LLVM File.td -opts) makes File.td relative to its own dir
(lib/Target/File.td) and not the targets'.

The only way I can think of solving this is to change how tablegen()
handles the file name, with some "if(IS_ABSOLUTE)", but looking
further, the file name is used to more than just a filename, and gets
included in some variables names, etc. Also, I can't add another
option to the end of tablegen() because, AFAICS, this function is
variadic, with all table-gen options following the file name...

Ideas?

--renato

cmake_tblgen_parser.patch (22.2 KB)

To be clear, TargetParser is about parsing subtarget CPUs and features,
right?

I'm not very familiar with how the current system works, but it would be a
shame to run tablegen for all targets when the user just wants one backend.

The subtarget feature flags aren't very much data. If I were doing this
today, I would make one subtarget info library and put all the subtarget
features in there. The subtarget information is far less than the
instruction table information. For example:
lib/Target/Subtargets/X86Subtargets.td
lib/Target/Subtargets/ARMSubtargets.td
...

Now that we have a lot of backends and lots of out of tree backends, this
might be too disruptive. The answer might be that we have to live with
duplicating the subtarget features in clang and building some kind of
sanity checking mechanism to keep them in sync.

To be clear, TargetParser is about parsing subtarget CPUs and features,
right?

In the first stage, yes. But there's a lot more. I hope this ends up
being a much larger infrastructure to query for target support, not
just parsing strings (which we can cope with duplication), but
defining architectural behaviour, which is a whole different case.

Clang is only one user. Ever other front-end has to do the same. All
other tools (lli, llc, etc) have to duplicate string parsing, and any
out-of-tree plugin will have to do the same, if they need anything
that is not built as a back-end.

There is a lot in Clang's driver that could be greatly simplified if
we had a proper unified target description.

I'm not very familiar with how the current system works, but it would be a
shame to run tablegen for all targets when the user just wants one backend.

Why? The only effect of this is to process all targets' table-gen
files at build time, mostly once for the targets you rarely use. They
will not be included in any library, binary or archive if you don't
use the specific back-end, and the TargetParser will only use the
necessary tables to get target information.

The subtarget feature flags aren't very much data. If I were doing this
today, I would make one subtarget info library and put all the subtarget
features in there. The subtarget information is far less than the
instruction table information. For example:
lib/Target/Subtargets/X86Subtargets.td
lib/Target/Subtargets/ARMSubtargets.td

It's not just CPU names. For instance, Clang has support for register
names. Right now, they're all accepted (x86 names for ARM code)
because Clang doesn't know better. Another example is assembler /
linker flags (-Wa,-Wl), which are not supported at all for the
integrated assembler and would be another user of parsing and
inspecting capabilities.

All the information is there, in the table-gen files, and could be
easily accessed into an abstraction layer (TargetDescription or
whatever), which TargetParser could use, as well as all other tools,
including lld, lldb, etc. We're not talking about duplication, we're
talking about multiplication by a factor a lot more than two.

cheers,
--renato

I suppose you mean “once per ‘make clean’ invocation”. Do you have any info on how long it takes to do this on a slow target, or on a windows machine?

Jim Rowan
jmr@codeaurora.org
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation

I suppose you mean “once per ‘make clean’ invocation”.

Or if any of the table-gen files change, yes.

Do you have any info on how long it takes to do this on a slow target, or on a windows machine?

On my ARM chromebook2, 4 Cortex-A15 cores, after llvm-tblgen has been
compiled (took 2min40s), both "ninja -j6 libLLVMARMInfo.a" and "ninja
-j6 libLLVMAArch64Info.a" up until the first cpp file took 14 seconds.

I assume other backends will have the same size / time. We have 13
back-ends, assuming you go from building one to building all 13,
you'll add 3 more minutes to the build. Bearing in mind that a full
build on that box takes around one hour, it's not that much, really.

This may not be the best way of doing it, but I don't think it's time
added enough, especially considering the enormous amount of complexity
that this will remove from Clang, LLVM, and all other tools. It may
even speed up the build, as well as allow us to implement sane
interfaces and APIs across the board.

cheers,
-renato

CMake turned out a lot more powerful than I was imagining, and the
change was really simple.

Let's follow it here:

http://reviews.llvm.org/D8195

The biggest question now is: do we have a consensus that building all
table-gen files is worth the massive de-duplication of current code
and future work that will follow?

cheers,
--renato

Agreed. That is not so significant that it should impact the choice.

Jim Rowan
jmr@codeaurora.org
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation

I’d like to avoid as much as possible adding compilation time to the process unless it is necessary (I’m not saying it does not worth it here).
It is not clear to me why do you need to generate *all* the table-gen files and not only the one related to target features?

Thanks,

Mehdi

I’d like to avoid as much as possible adding compilation time to the process unless it is necessary (I’m not saying it does not worth it here).

As I said in the review, it may end up being faster, because of the
amount of crap we'll remove from all tools and LLVM.

It is not clear to me why do you need to generate *all* the table-gen files and not only the one related to target features?

I don't know which of them yet, but I'll certainly need CPU/FPU/ARCH
names, register names, feature names and anything to populate a target
description object.

I moved them all into the include file because I didn't want to have
to choose now and get it wrong. I realise that this might not have
been the best course of action.

Having said that, we can always go back. So, I'm open to suggestions.
I could start with only a few per arch, and then add as we see fit. My
guess is that only ARMGenRegisterInfo.inc and ARMGenSubtargetInfo.inc
would be a good start, with possibly ARMGenCallingConv.inc and
ARMGenInstrInfo.inc being the next ones to add for Clang's benefit.

That would mean separating the calls to tablegen() between
CMakeLists.txt and CMakeTblgen.txt, but that's ok.

cheers,
--renato

I’d like to avoid as much as possible adding compilation time to the process unless it is necessary (I’m not saying it does not worth it here).

As I said in the review, it may end up being faster, because of the
amount of crap we'll remove from all tools and LLVM.

Only if you build all those tools :wink:
But yes that’s a good point overall.

It is not clear to me why do you need to generate *all* the table-gen files and not only the one related to target features?

I don't know which of them yet, but I'll certainly need CPU/FPU/ARCH
names, register names, feature names and anything to populate a target
description object.

I moved them all into the include file because I didn't want to have
to choose now and get it wrong. I realise that this might not have
been the best course of action.

Having said that, we can always go back. So, I'm open to suggestions.
I could start with only a few per arch, and then add as we see fit. My
guess is that only ARMGenRegisterInfo.inc and ARMGenSubtargetInfo.inc
would be a good start, with possibly ARMGenCallingConv.inc and
ARMGenInstrInfo.inc being the next ones to add for Clang's benefit.

That would mean separating the calls to tablegen() between
CMakeLists.txt and CMakeTblgen.txt, but that's ok.

I just re-timed and the recent improvements Owen and Chris made to table-gen turn the full generation of table-gen files for my out-of-tree backend from ~8min down to ~35s in debug builds, pretty nice.
At this point I’m no longer afraid about the potential compile time impact of this change.

Thanks for working on this.

Other than building it, I admit that I’m curious what sort of thing you’ve got planned for how to vend the information. Before we go further down this path can you provide a more detailed plan here?

-eric

Oh, did I have to do anything useful with it? :slight_smile:

Attached is a very crude patch where I stopped on the FPU parser.
That's when I realised that I didn't want to duplicate the tablegen
information not even once.

This patch was just me moving the ugly stuff under one common class,
but there was no way I could guarantee that the inc files would be
generated, and that would break the Clang promise of working all the
way to IR independent on which back-end is bundled with it.

I had a momentary opinion that maybe Clang didn't need to have that
promise, but when I realised that independent of that, I'd still have
to duplicate the knowledge on the fpu parser, and on the cpu parser,
and on the target info, so on and so on, it hit me that this is a lot
more than just Clang's ability to parse fpu strings.

This is the bug where I started brainstorming...

https://llvm.org/bugs/show_bug.cgi?id=20787

My idea is to have this wrapper class that will generate the target
classes (like TargetInfo, TargetMachine or whatever is needed by some
tool, future will tell), so that I don't need to duplicate them as
well. The parser will be just another accessory of such a
TargetDescription umbrella, one that Clang's front end (and the
integrated assembler, and llc, lli, etc) will use. Clang's code
generation might use TargetInfo, for example, when interpreting inline
assembly, more specifically named registers, so we can reject x86
register names when compiling for ARM.

The main problems are the difference in command line options between
clang->object and clang->IR->llc->asm->ld->object, or options passed
down the assembler and linker (-Wa, -Wl), and how Clang and LLVM have
to be independently informed about the target capabilities.

The first step, though, will be to only have TargetParser reading the
info part and guessing cpu/fpu/arch names for the parser.

Makes sense?

cheers,
--renato

fpuparser.patch (13.2 KB)

Gentle ping?