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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp:53
+
+  EM->setSection(".llvm.lto");
+  EM->setAlignment(Align(1));
----------------
Does this get assigned the correct section flags? I'd expect it to need SHF_EXCLUDE so it doesn't end up in the final binary when linked without LTO.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1461
+  MPM.addPass(EmbedBitcodePass(ThinLTOPreLink, EmitUseList, EmitSummary));
+  addPerModuleDefaultPipelinePasses(MPM, Level, /*LTOPreLink=*/false);
   return MPM;
----------------
paulkirth wrote:
> 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?
> 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.

Yes, I was using fat as in non-thin here :) It's unfortunate that that the term "fat LTO" can now refer to both "non-thin LTO" and "fat-object LTO", including the peculiar combination of "fat thin LTO"...

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

The way to observe this would be to try `-ffat-lto-object` together with something like `-fsanitize=address`. You should see address sanitizer getting run twice.

As to addressing it, I'm not sure there's a super clean way to do that short of adding an extra parameter to buildThinLTOPreLinkDefaultPipeline to suppress running the relevant extension points.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146776