[RFC] Use a Cache for Parallel FullLTO CodeGen

Hi,

We would like to add a caching system, similar to ThinLTO’s, for FullLTO’s parallel codegen feature.

Proposed Changes

Add a system that caches the output of TargetMachine::splitModule in LTOBackend.cpp:splitCodeGen.

This would be implemented the same way as the ThinLTO cache and reuse the same component (Support/Caching.h, Support/CachePruning.h). The main piece that is missing is how to get a stable module hash (see “Challenges” below).

Reasoning

This is very desirable for the AMDGPU backend, as the backend portion of the compiler is by far the most expensive (it’s generally where 2/3rds of the time is spent after the front-end, more in pathological cases). We are looking to optimize the backend in parallel, but that is a continuous effort with little low-hanging fruits left. AMDGPU is a very complex target, and the large amount of registers we have slow down RegisterCoalescer and RegAlloc quite a bit.

Another factor that does not help is that AMDGPU requires “whole module” compilation: we have to be able to see all functions called by a kernel to build the module. Thus, modules can get quite large (dozens if not hundreds of MBs in some cases), either due to the application’s structure, or because -fgpu-rdc (which enables Full LTO) is required. This is also something that we’re tracking, there are ongoing efforts to enable machine linking or maybe thin LTO (though neither of these are confirmed or have an ETA).

AMDGPUSplitModule was introduced fairly recently to help with big modules when using -fgpu-rdc, and it proved quite successful at that. We now propose enabling caching of module splitting results because it’s a fairly non-intrusive change that really helps in real-world scenarios, especially in the case of small incremental changes.

Challenges

How can we create stable LLVM IR module hashes for caching ? I believe the easiest way is to print the LLVM IR to a string and hash it, but I wonder if we could have issues with things like commit SHAs or module IDs constantly changing, and I am not sure how we could address that.

Thanks

I’m resuming conversation on this topic as this came up again, and I did not have time to work on it yet.

I looked at what it’d take to implement this, and this is the outline of it:

  • We can hash modules using StructuralHash, we’ll need to throw in some more details from the LTO Config by factoring out some common code from the ThinLTO module hash code.
    • We’ll likely start with super conservative hashes, using DetailedHash=true. After testing we can maybe relax it a bit.
    • Should the TargetMachine provide this in case targets want more control over how to hash ? I don’t think it’s necessary.
  • The FullLTO code path will need to pass a FileCache argument so it can reach the splitCodeGen helper.
    • If the FileCache is invalid, nothing happens, so there is little to no cost for others.
  • splitCodeGen needs to check if there is a cache, if there is one, get a module hash then check if there is a hit or not. If there is one, directly return the object file. Otherwise, run the back-end and write the object file to cache.
  • LLD will need a handful of new options to enable this. I think the following works:
    • Disable this by default, add a new pair of options for specifying a caching directory for Full LTO, and a set of Full LTO pruning option.
    • AMDGPU Clang driver passes the right options.

I think this keeps the changes at a minimum for the other targets in the linker while enabling this functionality for AMDGPU. Thus, I think we could do this upstream directly. Would there be any objections ?

cc @MaskRay

We’ll likely need to do some fuzzing/more testing off detailed structural hash before using it in a context like this. When I originally worked on it, I used it for dataset duplication that didn’t need to be incredibly accurate. I believe @yxsamliu had some changes from work on -print-changed that need to land (separately from that work), but I don’t believe they have at this point and there might be other holes.

1 Like

I’m a bit confused, it sounds like the proposal is to use StructuralHash with an assumption of “it the hash is the same, the module is the same”? That seems fundamentally wrong. Not only was StructuralHash never intended to capture all IR properties, but the hash size is also too small to reasonably assume absence of collisions.

StructuralHash could only be used to find a candidate, but module equivalence still needs to be established separately.

I think this is kind of the same discussion as here: Synchronizing LTO code-generation configuration between Clang and LLD (DTLTO)

Hashing module is not a hard problem as you pointed out, you just need to hash the llvm bitcode serialization of the module (just need to pay the overhead of the serialization, which might not be cheap). But in order to build a cache, you need LTOConfig also contributing to the cache key, thus the things like SHAs that you worried about is actually a must have and ensures the cache is correct.

In order to serialize/hash a LTOConfig, the other thread is a good place to take about it so it should just be the same solution for both.

3 Likes

I’m a bit confused, it sounds like the proposal is to use StructuralHash with an assumption of “it the hash is the same, the module is the same”? That seems fundamentally wrong

I didn’t look very hard into the hashing part yet. Good to know StructuralHash is not enough

Hashing module is not a hard problem as you pointed out, you just need to hash the llvm bitcode serialization of the module (just need to pay the overhead of the serialization, which might not be cheap)

I think the overhead is fine, and I agree; hashing the bitcode directly seems best.

The main use case we have in mind is for very large applications where the back-end portion takes something like 30s+, sometimes even minutes, per LTO partition. Serializing the bitcode is a perfectly acceptable overhead.


Before I put time into implementing this, are there any strong objections regarding the overall feature ? While this would be implemented in a generic way, AMDGPU is definitely the only target so far that’d need that. I know we want to avoid target-specific bits in generic code like that, I want to ensure this won’t get rejected on principles

Also, how should this feature interact with the ThinLTO cache ?

I think the easiest is to re-use it instead of maintaining an entirely separate cache. I can rename the options to remove the ThinLTO specific-ness in the names, and ensure the old names still work by adding aliases.

Alternatively, I can add another cache for this specific case.

Also, how should this feature interact with the ThinLTO cache ?

It would be nice to reuse the same infrastructure after splitModule. Then it is easier to maintain and can access to things like distributed ThinLTO pipeline easily.

Architecture generic will be preferred and hope it is not too big a problem since thinLTO cache and splitModule are both not architecture specific.

I’m on the fence about this. The ThinLTO cache is very specific (controlled by options that have “ThinLTO” in their name all over, documented as being for ThinLTO incremental compilation, etc.) - I don’t want to mess it with too much and make it more confusing.

In the spirit of keeping this as non-intrusive as possible, I feel like a separate cache is best, but it does duplicate a tiny bit of code in a few places, though not that much.

Looking into this even more, I just realized that a lot of Caching infra is thinlto-specific. For example Caching.cpp can emit warnings that mention ThinLTO directly.

I wonder what’s best here. Reuse all the ThinLTO-branded caching, and simply document that we can reuse the ThinLTO cache (e.g. through a CL opt such as lto-partitions-use-thinlto-cache=1), or split this up entirely into two distinct caches.

I am not familiar enough with the caching logic to know for sure if it can cause issues to reuse the cache for things other than ThinLTO modules. I’ll keep looking into this.

PR: [lld] Add caching for `--lto-partitions` by Pierre-vh · Pull Request #212203 · llvm/llvm-project · GitHub