[PATCH] D146776: [llvm] Preliminary fat-lto-objects support

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1414
+                                                    OptimizationLevel Level,
+                                                    bool LTOPreLink) {
+  assert(Level != OptimizationLevel::O0 &&
----------------
nikic wrote:
> Splitting out this method should not be necessary, you can add one MPM to another.
Noted. I'll take a look at refactoring this accordingly.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1461
+  MPM.addPass(EmbedBitcodePass(ThinLTOPreLink, EmitUseList, EmitSummary));
+  addPerModuleDefaultPipelinePasses(MPM, Level, /*LTOPreLink=*/false);
   return MPM;
----------------
nikic wrote:
> Running the full default pipeline after the pre-link pipeline doesn't make a lot of sense. You should only run the module optimization pipeline here. For ThinLTO, the pre-link pipeline + module optimization will give you pretty much exactly the default pipeline. For FatLTO, it will run module optimization twice, at least until D148010 lands.
> 
> This will still have some problems though, such as running the optimization pipeline extension points twice.
> Running the full default pipeline after the pre-link pipeline doesn't make a lot of sense. You should only run the module optimization pipeline here. For ThinLTO, the pre-link pipeline + module optimization will give you pretty much exactly the default pipeline. 

This is a very helpful detail, so thanks for pointing that out.

>For FatLTO, it will run module optimization twice, at least until D148010 lands.
> 

Just for clarification, when you say `FatLTO` are you referring to full/normal LTO? In my experience `FatLTO` has typically referred to the feature being implemented in this patch, since it uses a fat object file format for LTO purposes, and has been common in GCC for a long time. The only place I've seen `FatLTO` used otherwise is in the Rust compiler, so I want to be sure I'm understanding you correctly.

> This will still have some problems though, such as running the optimization pipeline extension points twice.

Do you have any recommendations about how we may address that? or at least suggestions on where to focus investigation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146776