Should we split llvm Support and ADT?

Project Infrastructure LLVM Dev List Archives
21

If the end result is “Base”, “Misc”, “Broad”, “Narrow”, “Util”, “Support” or other non-descriptive names then I’m against it!

If you can find pieces that can be broken out as a logical units such as “TargetTriple”(for Triple.h and all the ARM parsers) or “object file formats” then I don’t think anyone would object patches to do so. Motivating this with a better project structure sounds sensible to me, I am not convinced your buildtimes will change all that much: The next change to something like ADT/StringRef.h will come and you have to do the tablegen rebuild.

  • Matthias
22

Let me try starting over without mentioning Tablegen :slight_smile:

Is Support too big, and if so should we consider breaking it up?

So far it sounds like the general consensus is “yes”, with some quibbles about precisely how to split it.

I’m not necessarily opposed to smaller libraries like Triple as you suggest, but i think that only addresses what to with largely disparate set of stuff which I’m calling “narrowly useful”. It doesn’t really address the fact that even after doing it at every logical division point, there will still be a set of stuff that doesn’t belong with other stuff.

To address that, I’m proposing something like “Base” which is similar in spirit to Support as it exists today, but more restrictive, and must satisfy the “would this be useful outside of LLVM” bar.

23

I would like to better understand how you came to conclude that the tablegen re-runs based on changes in Support are what’s causing your build to be slow and what part specifically is taking all that time. I can do a clean release + assertions build of LLVM, Clang, compiler-rt and lld in about 5 minutes, plus 40 seconds to run cmake, on what I think is similar hardware to what Zach is using. If I swap Ptr += ret and Size -= Ret in raw_fd_ostream::write_impl so that Support changes, I can do an incremental build in about 10 seconds, including the regeneration of various .inc files. How do we get from there to 10 minutes for an incremental build?

24

Have you tried a Debug build with all the backends configured in? I think
part of the issue is that Tablegen suffers from being compiled in debug
mode (hence the OPTIMIZED_TABLE_GEN cmake option).
Also not everyone has a "google-like" workstation :wink:

25

I would like to better understand how you came to conclude that the tablegen re-runs based on changes in Support are what’s causing your build to be slow and what part specifically is taking all that time. I can do a clean release + assertions build of LLVM, Clang, compiler-rt and lld in about 5 minutes, plus 40 seconds to run cmake, on what I think is similar hardware to what Zach is using. If I swap Ptr += ret and Size -= Ret in raw_fd_ostream::write_impl so that Support changes, I can do an incremental build in about 10 seconds, including the regeneration of various .inc files. How do we get from there to 10 minutes for an incremental build?

Which build system are you using?

On my build config (16 core/32 thread, 128GB RAM) it takes 47 user seconds to rebuild the clang binary after making the change you mentioned - with ninja and lld, host compiler is a recent release build of clang. Ninja reports building 377 things.

Comparatively, if I change something in lib/CodeGen/AsmPrinter, it builds 4 things and takes 19 user seconds. (most of that’s probably the link step)

So it seems like rebuilding tablegen does rebuild quite a few things - but I’m not sure exactly which parts are avoidable and which parts aren’t.

26

Let me try starting over without mentioning Tablegen :slight_smile:

Is Support too big, and if so should we consider breaking it up?

So far it sounds like the general consensus is “yes”, with some quibbles about precisely how to split it.

I’ll dissent here - if there are observable/meaningful benefits in real-world build/iteration time, that seems like it’d motivate the work - without that, I don’t think there’s necessarily an optimal size for a library or that breaking up Support is especially valuable/good.

  • Dave
27

At least for me ninja starts with a high estimated number of files needing a rebuild. But the number of targets immediately jumps down by several hundreds as soon as ninja realizes that the tablegen output didn’t actually change.

  • Matthias
28

I would like to better understand how you came to conclude that the tablegen re-runs based on changes in Support are what’s causing your build to be slow and what part specifically is taking all that time. I can do a clean release + assertions build of LLVM, Clang, compiler-rt and lld in about 5 minutes, plus 40 seconds to run cmake, on what I think is similar hardware to what Zach is using. If I swap Ptr += ret and Size -= Ret in raw_fd_ostream::write_impl so that Support changes, I can do an incremental build in about 10 seconds, including the regeneration of various .inc files. How do we get from there to 10 minutes for an incremental build?

Which build system are you using?

On my build config (16 core/32 thread, 128GB RAM) it takes 47 user seconds to rebuild the clang binary after making the change you mentioned - with ninja and lld, host compiler is a recent release build of clang. Ninja reports building 377 things.

At least for me ninja starts with a high estimated number of files needing a rebuild. But the number of targets immediately jumps down by several hundreds as soon as ninja realizes that the tablegen output didn’t actually change.

Fair - I think the estimate did start in the thousand+ and 377 was the final number. So I guess it’s pruning to some degree - not sure exactly what/how.

29

For the sake of comparison, I made the same change and it took 1:58.39. A slightly different but more intrusive change to Format.h in format_object_base::print() took 5:6.54. A clean build on the same machine takes about 12 minutes.

I tried a few hacks to the CMake to say “don’t run tablegen, no matter what, and don’t make anything depend on tablegen’s output”, but I couldn’t come up with the right magic. If anyone knows, I can test a side-by-side comparison without tablegen.

30

I would like to better understand how you came to conclude that the tablegen re-runs based on changes in Support are what’s causing your build to be slow and what part specifically is taking all that time. I can do a clean release + assertions build of LLVM, Clang, compiler-rt and lld in about 5 minutes, plus 40 seconds to run cmake, on what I think is similar hardware to what Zach is using. If I swap Ptr += ret and Size -= Ret in raw_fd_ostream::write_impl so that Support changes, I can do an incremental build in about 10 seconds, including the regeneration of various .inc files. How do we get from there to 10 minutes for an incremental build?

For the sake of comparison, I made the same change and it took 1:58.39. A slightly different but more intrusive change to Format.h in format_object_base::print() took 5:6.54. A clean build on the same machine takes about 12 minutes.

Changing the header seems liable to rebuild a bunch of things that include the header - not sure any layering changes would change that, would they? (I don’t think ninja at least waits for the library to build before building things that include the library’s headers)

Out of curiosity - what build system/linker/host compiler/OS are you using? (& cores/ram/etc too)

I tried a few hacks to the CMake to say “don’t run tablegen, no matter what, and don’t make anything depend on tablegen’s output”, but I couldn’t come up with the right magic. If anyone knows, I can test a side-by-side comparison without tablegen.

That’s why I made an equivalent change above tablegen to see what the relative cost was - I think that’s probably pretty representative. Without tablegen touching any cpp file should (& with ninja, seems to) rebuild that object, the library, and the resulting binary & not much/anything else.

  • Dave
31

ninja, MSVC compiler, MSVC linker, Windows 10, 48 logical cores, 64GB ram.

32

I mentioned earlier in the thread that there is already a CMake option for
this: -DLLVM_TABLEGEN=path/to/llvm-tblgen

33

Doesn’t that just tell it what tablegen to use? I was looking for an option to make it not run anything, and just assume that all tablegen definitions were up to date

34

If you tell it to run an external llvm-tblgen, this external tablegen
becomes the producers of all the generated file. And since this external
tablegen binary is not produced by your CMake, it does not depends on any
source file.

35

But it will still re-write the output files, updating their timestamps and trigger rebuild of all dependencies (at worst) or a diff-and-noop (at best), no? Since the original proposal was about reducing the number of situations in which tablegen runs at all, I would think that the best comparison would be one in which tablegen doesn’t run at all.

36

Why would this happen? What would trigger that in the dependency tree?
The llvm-tblgen binary used to generate the output files cannot change...
(I may be missing something)

37

Set it to /bin/true or something like that. Then it won't overwrite anything.

-Krzysztof

38

Zach already mentioned that he really wants to focus on Support rather than tablegen, but I gathered some numbers so I figured I might as well share them. I did a few builds on Windows 10 with Clang as the compiler and link.exe as the linker (so not the exact same configuration as Zach, who was using the MSVC compiler). Here are the numbers:

Debug build with LLVM_OPTIMIZED_TABLEGEN=off:

clean build: about 10 minutes
rebuild after raw_ostream change: about 100 seconds

Debug build with LLVM_OPTIMIZED_TABLEGEN=on:

clean build: about 10 minutes
rebuild after raw_ostream change: about 50 seconds

RelWithDebInfo + asserts build with LLVM_OPTIMIZED_TABLEGEN=off:

clean build: about 9 minutes
rebuild after raw_ostream change: about 20 seconds

We can see that using a Debug version of tablegen really slows things down a lot. We go from 50 seconds to 100 seconds. And the 50 seconds already includes building tablegen’s dependencies twice - one optimized version for the optimized tablegen, and one debug version for the rest of the build.

My understanding is that we rebuild and re-run tablegen in response to changes in Support, and so this would re-run tablegen for everything, conclude that the generated files are the same as the previously generated files, and not overwrite the generated files. In other words, a build on the same hardware and OS that takes 5 or 10 minutes isn’t spending more time in tablegen than the 100-second build I did, and so spends most of its time doing things that are not tablegen.

39

Let me try starting over without mentioning Tablegen :slight_smile:

Aw, but i had a list of issues, such as:

  • tablegen doesn’t generate .d files
  • CMake is set up with far reaching deps on .td files, so changing Options.td rebuilds the backends, and adding an ARM intrinsic rebuilds X86 or vice versa
  • All the tablegen backends are linked in to a single binary, so if you are hacking on tablegen and change the disassemblers backend, you rebuild the SelectionDAG code

Is Support too big, and if so should we consider breaking it up?

Yes please. Even if it didn’t have a signifiant impact on build time, its become something of a dumping ground for code which would otherwise cause layering violations if put somewhere else.

The way I see it, there’s a bunch of functionality which effectively just wraps the platform, (i.e., anything which abstracts away posix/unix vs Windows vs macOS). This is often stuff like file systems, streams, the host itself and probably a few others. Then there is obviously ADT. There are utilities which basically everyone uses like Error, Debug, Statistic and stack trace. There are utilities which only tools should use, like CommandLine, and utilities only used by a few projects right now, but maybe more in future, like Threading.

Then there are things which should now live elsewhere because perhaps other libraries exist now which didn’t before. ELF.h and others should be in libObjectFile. If they can’t be there, then we should have a very good reason why not.

BranchProbability shouldn’t be there at all but probably is due to layering, and likely the same goes for TargetRegistry. We should fix that. Same goes for ARM* which should all be in the ARM backend. Dwarf should be in libDebugInfoDWARF. *DeltaAlgorithm aren’t even used outside of tests!

Anyway, sorry for the brain dump. But yes, this has grown in to something which can be cleaner. Perhaps Support will still exist to wrap posix and friends, hopefully with ADT separately, but we should also move a bunch of things to better suited libraries if we can.

Cheers,
Pete

40

Actually, it does if asked to.

Joerg