[PATCH] D152973: [gold] Add preliminary FatLTO support to the Gold plugin

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added inline comments.


================
Comment at: llvm/lib/Object/ObjectFile.cpp:82
   if (NameOrErr)
-    return *NameOrErr == ".llvmbc" || *NameOrErr == ".llvm.lto";
+    return *NameOrErr == ".llvm.lto";
   consumeError(NameOrErr.takeError());
----------------
BTW I feel like we should document this somewhere. I guess "Release Notes"? but maybe also some place else. Are there any suggestions to where that should live?


================
Comment at: llvm/test/LTO/X86/Inputs/bcsection.s:1
-.section .llvmbc
+.section .llvm.lto
 .incbin "bcsection.bc"
----------------
tejohnson wrote:
> Are we now missing a test for .llvmbc? Also, shouldn't .llvmbc and .llvm.lto be handled differently? I.e. the former ignored and the latter accepted and LTO linked (for which it should have a test similar to the lld one). Or is there meant to be a follow on patch to fully enable FatLTO handling in gold?
ah, yeah, I guess we would also want a negative test for `.llvmbc`. I'll add that. I was thinking that since the test was trying to LTO compile w/ `.llvmbc` section that just updating this to `.llvm.lto` would be sufficient.

>  Or is there meant to be a follow on patch to fully enable FatLTO handling in gold?

In my testing  with this patch gold handles things as I think would be expected. i.e. if given 2 `fat-lto-objects` files and passed `-plugin LLVMGold.so`, it will use the bitcode section in `.llvm.lto` to generate code. When linking from clang and passing `-fno-lto` the `-plugin LLVM.Gold.so` option isn't passed to the gold linker, and thus it uses the `.text` section. 

I can try to whip up a similar test to the LLD one here and test that behavior more thoroughly.

Off the top of your head, are there other test cases I should consider? I'd like to be thorough, but most of the tests I've thought of are some kind of layering violation, and should be (I believe) covered by existing tests in other layers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152973