Reland "[gold] Add preliminary FatLTO support to the Gold plugin""

[paulkirth]

Summary

This changes the definition if isSectionBitcode to only be valid for the
.llvm.lto section, since this API is only called from LTO, and the
.llvmbc section was not intended to be used for LTO. This allows the
gold plugin to keep its existing behavior without introducing any
significant changes.

[tejohnson]

llvm/test/LTO/X86/Inputs/bcsection.s

1 ↗

(On Diff #531548)

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?

[paulkirth]

llvm/lib/Object/ObjectFile.cpp

82

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?

llvm/test/LTO/X86/Inputs/bcsection.s

1 ↗

(On Diff #531548)

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.

[paulkirth]

Rebase and add some tests for .llvmbc.

I still need to add a test similar to the LLD one

[paulkirth]

Add test case similar to the LLD one.

I still need to create some tests for the .llvmbc section.

[paulkirth]

Rebase.

[MaskRay]

How does gold recognize .llvm.lto section for LTO?

llvm/test/LTO/X86/Inputs/llvm.lto.section.s

2

This is a small extra file. You can use split-file to avoid the extra file.

.section .llvm.lto,"e"

llvm/test/tools/gold/X86/fatlto/fatlto.test

5

As in the lld patch, ; RUN: rm -rf %t && mkdir %t && cd %t

12

readonly,exclude

56

As in the lld patch, just use cmp to compare object files.

[MaskRay]

I see. In claim_file_hook, Expected<std::unique_ptr<InputFile>> ObjOrErr = InputFile::create(BufferRef); transitively calls findBitcodeInObject, which finds .llvm.lto.

[paulkirth]

Rebase

[MaskRay]

We probably should support -?-plugin-opt=(no-)?fat-lto-objects to parallel the lld option.

[paulkirth]

Use similar tests to LLD

[paulkirth]

Fix some test issues.

• Remove REQUIRES
• Remove --fat-lto-objects from command lines
• Fix ordering of objects in the archive case

[paulkirth]

In D152973#4457524, @MaskRay wrote:

We probably should support -?-plugin-opt=(no-)?fat-lto-objects to parallel the lld option.

So, I'm not sure on that. On one hand, I think it makes sense from a consistency standpoint. On the other hand, the llvm gold plugin is already added when the user wants LTO, and will give select the bitcode sections for them. We added the --(no-)fat-lto-objects flag in LLD in part because there isn't an equivalent LTO on/off switch. Using the plugin at all seems to serve the same purpose to me.

Also, I'm not really sure how/why that would be desirable. Is the idea so you can use regular (Thin)LTO objects + FatLTO objects, but only choose the .text of the fat object? In my experiments GCC would link the fat objects w/ LTO unless you passed -fno-lto, but maybe that isn't an important consideration.

Supporting those meaningfully may also be a bit challenging, since it appears to me that we have less ability to change the behavior here than we do in LLD. But maybe I'm overthinking that part, and just plumbing a flag through to InputFile::create(and whatever else) is enough?

llvm/test/LTO/X86/Inputs/llvm.lto.section.s

2

I think it makes sense to keep this as is, since it is more or less the same as the existing bcsection.s.

If you feel strongly, then I can add a follow up patch to do so, and handle the related input files in a similar way.

[MaskRay]

In D152973#4498717, @paulkirth wrote:

In D152973#4457524, @MaskRay wrote:

We probably should support -?-plugin-opt=(no-)?fat-lto-objects to parallel the lld option.

So, I'm not sure on that. On one hand, I think it makes sense from a consistency standpoint. On the other hand, the llvm gold plugin is already added when the user wants LTO, and will give select the bitcode sections for them. We added the --(no-)fat-lto-objects flag in LLD in part because there isn't an equivalent LTO on/off switch. Using the plugin at all seems to serve the same purpose to me.

Also, I'm not really sure how/why that would be desirable. Is the idea so you can use regular (Thin)LTO objects + FatLTO objects, but only choose the .text of the fat object? In my experiments GCC would link the fat objects w/ LTO unless you passed -fno-lto, but maybe that isn't an important consideration.

Supporting those meaningfully may also be a bit challenging, since it appears to me that we have less ability to change the behavior here than we do in LLD. But maybe I'm overthinking that part, and just plumbing a flag through to InputFile::create(and whatever else) is enough?

OK, if supporting avoiding LTO for a relocatable object file with .llvm.lto is too difficult, this change is fine.

[paulkirth]

Update tests

• check that error: is in the error message
• avoid keeping the module summary in IR and generate it w/ opt --module-summary

[MaskRay]

llvm/test/tools/gold/X86/fatlto/fatlto.invalid.s

2 ↗

(On Diff #541220)

This should be gold?

10 ↗

(On Diff #541220)

delete blank line

[paulkirth]

Fix test so that it actually uses gold instead of lld.

[tbaeder]

The tools/gold/X86/fatlto/fatlto.test fails for us on everything but x86_64:

******************** TEST 'LLVM :: tools/gold/X86/fatlto/fatlto.test' FAILED ********************
Script:
--
: 'RUN: at line 3';   rm -rf /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp && split-file /builddir/build/BUILD/llvm-17.0.0.src/test/tools/gold/X86/fatlto/fatlto.test /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp
: 'RUN: at line 6';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llc /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-LTO.ll --filetype=obj -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.o
: 'RUN: at line 7';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/opt --module-summary /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-LTO.ll -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.bc
: 'RUN: at line 8';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-objcopy --add-section=.llvm.lto=/builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.bc /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.o
: 'RUN: at line 9';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-objcopy --set-section-flags=.llvm.lto=readonly,exclude /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.o
: 'RUN: at line 10';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-readobj -S /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.o | /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/FileCheck --check-prefix=CHECK-A /builddir/build/BUILD/llvm-17.0.0.src/test/tools/gold/X86/fatlto/fatlto.test
: 'RUN: at line 14';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llc /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-LTO.ll --filetype=obj -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.o
: 'RUN: at line 15';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/opt --module-summary /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-LTO.ll -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.bc
: 'RUN: at line 16';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-objcopy --add-section=.llvm.lto=/builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.bc /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.o
: 'RUN: at line 17';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-objcopy --set-section-flags=.llvm.lto=readonly,exclude /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.o
: 'RUN: at line 18';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-readobj -S /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.o | /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/FileCheck --check-prefix=CHECK-MAIN /builddir/build/BUILD/llvm-17.0.0.src/test/tools/gold/X86/fatlto/fatlto.test
: 'RUN: at line 23';   /usr/bin/ld.gold -plugin /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/lib64/LLVMgold.so -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatLTO /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.o
: 'RUN: at line 24';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-readobj -S /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatLTO | /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/FileCheck --check-prefix=CHECK-LTO-TARGET /builddir/build/BUILD/llvm-17.0.0.src/test/tools/gold/X86/fatlto/fatlto.test
: 'RUN: at line 27';   /usr/bin/ld.gold -plugin /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/lib64/LLVMgold.so -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatLTO.start_lib --start-lib /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.o --end-lib
: 'RUN: at line 28';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-readobj -S /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatLTO.start_lib | /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/FileCheck --check-prefix=CHECK-LTO-TARGET /builddir/build/BUILD/llvm-17.0.0.src/test/tools/gold/X86/fatlto/fatlto.test
: 'RUN: at line 34';   /usr/bin/ld.gold -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatNoLTO /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-fatLTO.o
: 'RUN: at line 35';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-readobj -S /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatNoLTO | /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/FileCheck --check-prefix=CHECK-NON-LTO-TARGET /builddir/build/BUILD/llvm-17.0.0.src/test/tools/gold/X86/fatlto/fatlto.test
: 'RUN: at line 42';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/opt --module-summary /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-LTO.ll -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-LTO.bc
: 'RUN: at line 43';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/opt --module-summary /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-LTO.ll -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-LTO.bc
: 'RUN: at line 44';   /usr/bin/ld.gold -plugin /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/lib64/LLVMgold.so -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-LTO /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-LTO.bc /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-LTO.bc
: 'RUN: at line 45';   cmp /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatLTO /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-LTO
: 'RUN: at line 50';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llc /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-LTO.ll --filetype=obj -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a.o
: 'RUN: at line 51';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llc /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-LTO.ll --filetype=obj -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main.o
: 'RUN: at line 53';   /usr/bin/ld.gold -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-noLTO /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a.o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main.o
: 'RUN: at line 54';   cmp /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatNoLTO /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-noLTO
: 'RUN: at line 57';   /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/bin/llvm-ar rcs /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a.a /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a-fatLTO.o
: 'RUN: at line 58';   /usr/bin/ld.gold -plugin /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/lib64/LLVMgold.so -o /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatLTO.archive /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/main-LTO.bc /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/a.a
: 'RUN: at line 59';   cmp /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-fatLTO.archive /builddir/build/BUILD/llvm-17.0.0.src/redhat-linux-build/test/tools/gold/X86/fatlto/Output/fatlto.test.tmp/foo-LTO
--
Exit Code: 1

Command Output (stderr):
--
/usr/bin/ld.gold: error: /tmp/lit-tmp-_vrw63qp/lto-llvm-fd907e.o: incompatible target

[peter.smith]

The documentation in https://llvm.org/docs/BitCodeFormat.html#native-object-file-wrapper-format says:

Bitcode files for LLVM IR may also be wrapped in a native object file (i.e. ELF, COFF, Mach-O). The bitcode must be stored in a section of the object file named __LLVM,__bitcode for MachO and .llvmbc for the other object formats. This wrapper format is useful for accommodating LTO in compilation pipelines where intermediate objects must be native object files which contain metadata in other sections.

Not all tools support this format. For example, lld and the gold plugin will ignore these sections when linking object files.

We noticed this as this change has broken our downstream LTO bitcode wrapper as it was using .llvmbc as a section name. I'm sure we can adapt, but would be good to clarify this section of the reference manual.

[paulkirth]

@tbaeder, could you provide a bit more information about the configuration you are using? The test is in an x86 directory, which should mean it only runs on x86 enabled targets, right? If this is blocking you, I'm OK w/ a revert, but do want to understand what the issue is.

@peter.smith thank you for pointing that out. I will be sure to update the documentation.

[nikic]

In D152973#4519148, @paulkirth wrote:

@tbaeder, could you provide a bit more information about the configuration you are using? The test is in an x86 directory, which should mean it only runs on x86 enabled targets, right? If this is blocking you, I'm OK w/ a revert, but do want to understand what the issue is.

The test requires not only the x86 target in LLVM, but also that ld.gold supports it, which will generally only be the case on x86 hosts. So I think the test needs something like REQUIRES: x86_64-linux (not sure if that's the right spelling...).

@peter.smith thank you for pointing that out. I will be sure to update the documentation.

FYI this change also broke Rust LTO for the same reason (.llvmbc used for its own fat LTO equivalent). I expect that can be fixed by switching it to use .llvm.lto, but not sure if that will cause any other issues.

[paulkirth]

In D152973#4520240, @nikic wrote:

In D152973#4519148, @paulkirth wrote:

@tbaeder, could you provide a bit more information about the configuration you are using? The test is in an x86 directory, which should mean it only runs on x86 enabled targets, right? If this is blocking you, I'm OK w/ a revert, but do want to understand what the issue is.

The test requires not only the x86 target in LLVM, but also that ld.gold supports it, which will generally only be the case on x86 hosts. So I think the test needs something like REQUIRES: x86_64-linux (not sure if that's the right spelling...).

I see. Thank you for pointing that out. I think I may just revert this for now while we hash out the potential issues downstream.

@peter.smith thank you for pointing that out. I will be sure to update the documentation.

FYI this change also broke Rust LTO for the same reason (.llvmbc used for its own fat LTO equivalent). I expect that can be fixed by switching it to use .llvm.lto, but not sure if that will cause any other issues.

Hmm, I wasn't aware that rustc used a .llvmbc section. Thanks for pointing this out .The initial discussion on discourse pointed out that .llvmbc was only added to support the apple use case (discussion on this point starts roughly here https://discourse.llvm.org/t/rfc-ffat-lto-objects-support/63977/3), and non-lto usage should go away. We moved to a .llvm.lto section to make the distinction clear that this was LTO compatible bitcode and not just whatever the frontend emitted.

In your opinion, do we need a soft transition here? If so, any advice on what that should look like would be helpful.

[paulkirth]

Add REQUIRES lines to gold tests and update summary

[paulkirth]

Update docs on bitcode sections in object files to describe the new .llvm.lto section and how it is intended to differ from .llvmbc.

[nikic]

llvm/docs/BitCodeFormat.rst

479 ↗

(On Diff #543607)

Do we? This documentation seems to be the only reference to __lto I can find.

isSectionBitcode() for MachoOObjectFile only checks for __LLVM,__bitcode.

[paulkirth]

@peter.smith @nikic @MaskRay @tejohnson, given that Rust and others seem to use the .llvmbc, I see a few ways we can move forward here.

1. have a hard transition (e..g., this patch), and make downstream consumers adapt now.
2. have a soft transition, and mark .llvmbc as deprecated, then remove in the following release (so about 6 months from now).
3. forgo trying to separate the old uses of .llvmbc from new FatLTO uses and just reuse the .llvmbc section name.
4. ... something completely different?

I'm not really sure what the best way to go about this would be. I'd lean towards a soft transition (2), since that seems more forgiving and less hostile towards downstream consumers, but I think there are merits for any of the approaches. There are probably other options I haven't considered either, so thoughts and suggestions would be quite welcome.

[paulkirth]

llvm/docs/BitCodeFormat.rst

479 ↗

(On Diff #543607)

You are correct. I haven't implemented MachO support yet. I was over zealous trying to update this, and that is currently incorrect.

[paulkirth]

llvm/docs/BitCodeFormat.rst

479 ↗

(On Diff #543607)

As an FYI to reviewers, I will wait on rewording this again until we reach a consensus on how to proceed w/ the transition.

[MaskRay]

In D152973#4529625, @paulkirth wrote:

@peter.smith @nikic @MaskRay @tejohnson, given that Rust and others seem to use the .llvmbc, I see a few ways we can move forward here.

1. have a hard transition (e..g., this patch), and make downstream consumers adapt now.
2. have a soft transition, and mark .llvmbc as deprecated, then remove in the following release (so about 6 months from now).
3. forgo trying to separate the old uses of .llvmbc from new FatLTO uses and just reuse the .llvmbc section name.
4. ... something completely different?

I'm not really sure what the best way to go about this would be. I'd lean towards a soft transition (2), since that seems more forgiving and less hostile towards downstream consumers, but I think there are merits for any of the approaches. There are probably other options I haven't considered either, so thoughts and suggestions would be quite welcome.

If -ffat-lto-objects doesn't need to go to the LLVM 17.0.0 release, we can do a soft transition:

• Keep the LLVM and Clang parts in the release/17.x branch
• Revert the Clang Driver -ffat-lto-objects change in release/17.x so that Clang 17 still doesn't support -ffat-lto-objects.
• Give some time for @peter.smith and https://github.com/rust-lang/rust/ to migrate to .llvm.lto . I am not familiar with rust-lang/rust, but it seems that @alexcrichton is involved in some changes. Note, giving some time for the downstream should be considered a courtesy, not a norm.

I am curious how downstream users perceive a breakage? Is any of IRObjectFile::findBitcodeInObject, IRObjectFile::findBitcodeInMemBuffe, lto_module_is_object_file_for_target used?

If a C API for finding .llvm.lto is needed, we can add one and even back port it into release/17.x.

[nikic]

From a Rust perspective, I'm reasonably confident that we can deal with the patch as-is. We currently use findBitcodeInMemBuffer(), but we should easily be able to replace that with our own implementation. (I suspect we'll continue using the llvmbc section for the time being, specifically so only our own LTO implementation picks it up, which would match previous behavior where llvmbc was not used by linker plugin LTO or lld.)

The timing here is unfortunate, but I would prefer to land both the lld and gold changes before LLVM 17 branches, so we're not left with half-implemented fat object support.

[paulkirth]

In D152973#4529666, @MaskRay wrote:

If -ffat-lto-objects doesn't need to go to the LLVM 17.0.0 release, we can do a soft transition:

Given that Rust pins the LLVM revision to a stable release I think it may be good to get it into Clang 17. Doubly so, since it seems to me that they already have a version of FatLTO, and would benefit from using the FatLTO pipeline.

• Keep the LLVM and Clang parts in the release/17.x branch
• Revert the Clang Driver -ffat-lto-objects change in release/17.x so that Clang 17 still doesn't support -ffat-lto-objects.
• Give some time for @peter.smith and https://github.com/rust-lang/rust/ to migrate to .llvm.lto . I am not familiar with rust-lang/rust, but it seems that @alexcrichton is involved in some changes. Note, giving some time for the downstream should be considered a courtesy, not a norm.

Thanks for the suggestion. If we're not trying to make the next release, this sounds reasonable to me, though I am curious to hear what others think, too.

[MaskRay]

In D152973#4529730, @paulkirth wrote:

In D152973#4529666, @MaskRay wrote:

If -ffat-lto-objects doesn't need to go to the LLVM 17.0.0 release, we can do a soft transition:

Given that Rust pins the LLVM revision to a stable release I think it may be good to get it into Clang 17. Doubly so, since it seems to me that they already have a version of FatLTO, and would benefit from using the FatLTO pipeline.

My suggestion is based on my (possibly wrong) understanding that rust-lang/rust only needs the LLVM side change, which is in release/17.x, but not Clang or lld changes :)
That said, if people feel that it's good to ensure clang and lld changes are in release/17.x, I'm totally fine as well.

(I'll go on a trip from Thursday onwards and won't be able to respond in time. Apologies if anyone needs me to do anything.)

[nikic]

In D152973#4529730, @paulkirth wrote:

In D152973#4529666, @MaskRay wrote:

If -ffat-lto-objects doesn't need to go to the LLVM 17.0.0 release, we can do a soft transition:

Given that Rust pins the LLVM revision to a stable release I think it may be good to get it into Clang 17. Doubly so, since it seems to me that they already have a version of FatLTO, and would benefit from using the FatLTO pipeline.

I don't think we are going to use the FatLTO pipeline as currently implemented in Rust, due to its bad compile-time characteristics. We need a FatLTO implementation that does not require compiling the module twice.

[paulkirth]

In D152973#4529866, @nikic wrote:

In D152973#4529730, @paulkirth wrote:

In D152973#4529666, @MaskRay wrote:

If -ffat-lto-objects doesn't need to go to the LLVM 17.0.0 release, we can do a soft transition:

Given that Rust pins the LLVM revision to a stable release I think it may be good to get it into Clang 17. Doubly so, since it seems to me that they already have a version of FatLTO, and would benefit from using the FatLTO pipeline.

I don't think we are going to use the FatLTO pipeline as currently implemented in Rust, due to its bad compile-time characteristics. We need a FatLTO implementation that does not require compiling the module twice.

That's unfortunate, and is something I hope we can address moving forward. I vaguely recall your suggestion that after D148010 we could probably do better w.r.t. optimizing the pipeline, is that still correct? Also, out of curiosity, how does rustc avoid the edge cases brought up in the initial patches, like missing certain transforms for ThinLTO + SamplePGO?

[nikic]

In D152973#4529995, @paulkirth wrote:

In D152973#4529866, @nikic wrote:

I don't think we are going to use the FatLTO pipeline as currently implemented in Rust, due to its bad compile-time characteristics. We need a FatLTO implementation that does not require compiling the module twice.

That's unfortunate, and is something I hope we can address moving forward. I vaguely recall your suggestion that after D148010 we could probably do better w.r.t. optimizing the pipeline, is that still correct?

It would at least make things a good bit simpler, by removing thin vs full LTO differences from the equation.

Also, out of curiosity, how does rustc avoid the edge cases brought up in the initial patches, like missing certain transforms for ThinLTO + SamplePGO?

I'm not sure Rust even supports SamplePGO. Maybe there is an unstable feature for it.

But the truth is that Rust doesn't have a dedicated fat LTO pipeline at all, it just runs the usual pipeline and then both embeds the final bitcode and emits object code. This is of course bad (for the same reasons the current full LTO pipeline is bad), and replacing it with a proper fat LTO pipeline would be great.

[peter.smith]

In D152973#4529625, @paulkirth wrote:

@peter.smith @nikic @MaskRay @tejohnson, given that Rust and others seem to use the .llvmbc, I see a few ways we can move forward here.

1. have a hard transition (e..g., this patch), and make downstream consumers adapt now.
2. have a soft transition, and mark .llvmbc as deprecated, then remove in the following release (so about 6 months from now).
3. forgo trying to separate the old uses of .llvmbc from new FatLTO uses and just reuse the .llvmbc section name.
4. ... something completely different?

I'm not really sure what the best way to go about this would be. I'd lean towards a soft transition (2), since that seems more forgiving and less hostile towards downstream consumers, but I think there are merits for any of the approaches. There are probably other options I haven't considered either, so thoughts and suggestions would be quite welcome.

I think we can adapt fairly quickly as it is an embedded toolchain and we don't support mixing LTO objects from previous versions, I think we'll be able to cope with an instant change. May be worth an RFC/FYI on Discourse as some downstream projects merge with upstream less frequently than we do.

[paulkirth]

In D152973#4531264, @nikic wrote:

In D152973#4529995, @paulkirth wrote:

In D152973#4529866, @nikic wrote:

I don't think we are going to use the FatLTO pipeline as currently implemented in Rust, due to its bad compile-time characteristics. We need a FatLTO implementation that does not require compiling the module twice.

That's unfortunate, and is something I hope we can address moving forward. I vaguely recall your suggestion that after D148010 we could probably do better w.r.t. optimizing the pipeline, is that still correct?

It would at least make things a good bit simpler, by removing thin vs full LTO differences from the equation.

Right, I had forgotten that patch is only about smoothing out the differences between the two pipelines. I suppose that if there are not major differences, then we could potentially use the UnifiedLTO pipeline and then (I think) we'd be able to just do the module simplification (+ anything needed for instrumentation) for generating the bitcode , and then run module optimization after to generate the object code. At least thats the rough idea in my head ATM. There are no doubt some unfortunate edge-cases, but I'm hoping we sort those out in the not too distant future.

Also, out of curiosity, how does rustc avoid the edge cases brought up in the initial patches, like missing certain transforms for ThinLTO + SamplePGO?

I'm not sure Rust even supports SamplePGO. Maybe there is an unstable feature for it.

Indeed, I see a -Z profile-sample--use=val flag in the help. Seems to have landed in https://github.com/rust-lang/rust/commit/a17193dbb931ea0c8b66d82f640385bce8b4929a, a couple years ago.

But the truth is that Rust doesn't have a dedicated fat LTO pipeline at all, it just runs the usual pipeline and then both embeds the final bitcode and emits object code. This is of course bad (for the same reasons the current full LTO pipeline is bad), and replacing it with a proper fat LTO pipeline would be great.

Agreed. I'd be very happy if this simplifies things for Rust. It's an important usecase for Fuchsia, so the sooner we can get support into rustc the better.

In D152973#4532043, @peter.smith wrote:

In D152973#4529625, @paulkirth wrote:

@peter.smith @nikic @MaskRay @tejohnson, given that Rust and others seem to use the .llvmbc, I see a few ways we can move forward here.

1. have a hard transition (e..g., this patch), and make downstream consumers adapt now.
2. have a soft transition, and mark .llvmbc as deprecated, then remove in the following release (so about 6 months from now).
3. forgo trying to separate the old uses of .llvmbc from new FatLTO uses and just reuse the .llvmbc section name.
4. ... something completely different?

I'm not really sure what the best way to go about this would be. I'd lean towards a soft transition (2), since that seems more forgiving and less hostile towards downstream consumers, but I think there are merits for any of the approaches. There are probably other options I haven't considered either, so thoughts and suggestions would be quite welcome.

I think we can adapt fairly quickly as it is an embedded toolchain and we don't support mixing LTO objects from previous versions, I think we'll be able to cope with an instant change. May be worth an RFC/FYI on Discourse as some downstream projects merge with upstream less frequently than we do.

OK, well, it seems like maybe we should just land these now, since it sounds like both downstream consumers will be able to adapt w/o much trouble. A post on Discourse is a good idea. I should be able to do that later today.

So, if there are no concerns, are we all OK w/ landing this again modulo documentation changes?

[paulkirth]

Revise documentation of bitcode sections

[peter.smith]

I'm happy for this to be relanded modulo documentation updates.

[paulkirth]

llvm/docs/BitCodeFormat.rst

473 ↗

(On Diff #544004)

@peter.smith @nikic I've gone ahead and revised this, since it seems we're OK w/ relanding now. Are we happy w/ the new text?

[paulkirth]

ping. Are we OK w/ landing this now, or are there other changes we want to adopt?

[paulkirth]

Ping. @nikic @peter.smith do we still need changes to the verbiage in the documentation? I've also posted an FYI in discourse about 2 weeks ago. https://discourse.llvm.org/t/planned-changes-to-object-file-support-for-embeded-bitcode-sections-in-elf-files/72546

Are there any remaining concerns? or is this OK to reland?

[nikic]

LGTM, sorry for the delay.

However, please file a revert for clang fat LTO support for the 17.x release branch, as this patch should not be backported, so we need to revert the other instead.

[paulkirth]

I've filed https://github.com/llvm/llvm-project/issues/64508 to address the backporting. I think that's following the correct guidelines, but I'm a bit unclear on the process between what is in our docs and what has been posted on discourse.

llvm/lib/Object/ObjectFile.cpp

Error ObjectFile::printSymbolName(raw_ostream &OS, DataRefImpl Symb) const {
   return Error::success();
 }
 
 uint32_t ObjectFile::getSymbolAlignment(DataRefImpl DRI) const { return 0; }
 
 bool ObjectFile::isSectionBitcode(DataRefImpl Sec) const {
   Expected<StringRef> NameOrErr = getSectionName(Sec);
   if (NameOrErr)
-    return *NameOrErr == ".llvmbc" || *NameOrErr == ".llvm.lto";
+    return *NameOrErr == ".llvm.lto";
   consumeError(NameOrErr.takeError());
   return false;
 }
 
 bool ObjectFile::isSectionStripped(DataRefImpl Sec) const { return false; }
 
 bool ObjectFile::isBerkeleyText(DataRefImpl Sec) const {
   return isSectionText(Sec);

llvm/test/LTO/X86/Inputs/bcsection.macho.s

 .section __FOO,__bitcode
 .asciz "Wrong Section"
 
 .section __LLVM,__bitcode
-.incbin "bcsection.bc"
+.incbin "llvm.lto.section.bc"

llvm/test/LTO/X86/Inputs/llvm.lto.section.s

+.section .llvm.lto
+.incbin "llvm.lto.section.bc"

llvm/test/LTO/X86/bcsection.ll

 ; RUN: rm -rf %t && mkdir -p %t
 ; RUN: llvm-as -o %t/bcsection.bc %s
 
 ; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-pc-win32 -o %t/bcsection.coff.bco %p/Inputs/bcsection.s
-; RUN: llvm-nm %t/bcsection.coff.bco | FileCheck %s
-; RUN: llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/bcsection.coff.o %t/bcsection.coff.bco
-; RUN: llvm-nm %t/bcsection.coff.o | FileCheck %s
+; RUN: llvm-nm %t/bcsection.coff.bco | FileCheck %s --allow-empty
+; RUN: not llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/bcsection.coff.o %t/bcsection.coff.bco
 
 ; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-unknown-linux-gnu -o %t/bcsection.elf.bco %p/Inputs/bcsection.s
-; RUN: llvm-nm %t/bcsection.elf.bco | FileCheck %s
-; RUN: llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/bcsection.elf.o %t/bcsection.elf.bco
-; RUN: llvm-nm %t/bcsection.elf.o | FileCheck %s
-
-; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-apple-darwin11 -o %t/bcsection.macho.bco %p/Inputs/bcsection.macho.s
-; RUN: llvm-nm %t/bcsection.macho.bco | FileCheck %s
-; RUN: llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/bcsection.macho.o %t/bcsection.macho.bco
-; RUN: llvm-nm %t/bcsection.macho.o | FileCheck %s
+; RUN: llvm-nm %t/bcsection.elf.bco | FileCheck %s --allow-empty
+; RUN: not llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/bcsection.elf.o %t/bcsection.elf.bco
 
 target triple = "x86_64-unknown-linux-gnu"
 
-; CHECK: main
+;; The .llvmbc section is not intended for use with LTO, so there should be nothing here
+; CHECK-NOT: main
 define i32 @main() {
   ret i32 0
 }

llvm/test/LTO/X86/llvm.lto.section.ll

 ; RUN: rm -rf %t && mkdir -p %t
-; RUN: llvm-as -o %t/bcsection.bc %s
+; RUN: llvm-as -o %t/llvm.lto.section.bc %s
+
+; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-pc-win32 -o %t/llvm.lto.section.coff.bco %p/Inputs/llvm.lto.section.s
+; RUN: llvm-nm %t/llvm.lto.section.coff.bco | FileCheck %s
+; RUN: llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/llvm.lto.section.coff.o %t/llvm.lto.section.coff.bco
+; RUN: llvm-nm %t/llvm.lto.section.coff.o | FileCheck %s
+
+; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-unknown-linux-gnu -o %t/llvm.lto.section.elf.bco %p/Inputs/llvm.lto.section.s
+; RUN: llvm-nm %t/llvm.lto.section.elf.bco | FileCheck %s
+; RUN: llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/llvm.lto.section.elf.o %t/llvm.lto.section.elf.bco
+; RUN: llvm-nm %t/llvm.lto.section.elf.o | FileCheck %s
 
-; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-pc-win32 -o %t/bcsection.coff.bco %p/Inputs/bcsection.s
-; RUN: llvm-nm %t/bcsection.coff.bco | FileCheck %s
-; RUN: llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/bcsection.coff.o %t/bcsection.coff.bco
-; RUN: llvm-nm %t/bcsection.coff.o | FileCheck %s
-
-; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-unknown-linux-gnu -o %t/bcsection.elf.bco %p/Inputs/bcsection.s
-; RUN: llvm-nm %t/bcsection.elf.bco | FileCheck %s
-; RUN: llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/bcsection.elf.o %t/bcsection.elf.bco
-; RUN: llvm-nm %t/bcsection.elf.o | FileCheck %s
 
 ; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-apple-darwin11 -o %t/bcsection.macho.bco %p/Inputs/bcsection.macho.s
 ; RUN: llvm-nm %t/bcsection.macho.bco | FileCheck %s
 ; RUN: llvm-lto -exported-symbol=main -exported-symbol=_main -o %t/bcsection.macho.o %t/bcsection.macho.bco
 ; RUN: llvm-nm %t/bcsection.macho.o | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
 
 ; CHECK: main
 define i32 @main() {
   ret i32 0
 }

llvm/test/tools/gold/X86/fatlto/Inputs/a-LTO.ll

+; ModuleID = 'a-LTO.bc'
+source_filename = "a.c"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i32 @a() #0 {
+entry:
+  ret i32 0
+}
+
+attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"PIC Level", i32 2}
+!2 = !{i32 7, !"PIE Level", i32 2}
+!3 = !{i32 7, !"uwtable", i32 2}
+!4 = !{i32 7, !"frame-pointer", i32 2}
+!5 = !{i32 1, !"ThinLTO", i32 0}
+!6 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+!7 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git 2288ebbec148ce8000fc2c1f63a4033cd847669e)"}
+
+^0 = module: (path: "a-LTO.bc", hash: (0, 0, 0, 0, 0))
+^1 = gv: (name: "a", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 1, live: 0, dsoLocal: 1, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 1, alwaysInline: 0, noUnwind: 1, mayThrow: 0, hasUnknownCall: 0, mustBeUnreachable: 0)))) ; guid = 12157170054180749580
+^2 = flags: 8
+^3 = blockcount: 1

llvm/test/tools/gold/X86/fatlto/Inputs/main-LTO.ll

+; ModuleID = 'main-LTO.bc'
+source_filename = "main.c"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i32 @main() #0 {
+entry:
+  %retval = alloca i32, align 4
+  store i32 0, ptr %retval, align 4
+  %call = call i32 (...) @a()
+  ret i32 %call
+}
+
+declare i32 @a(...) #1
+
+attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"PIC Level", i32 2}
+!2 = !{i32 7, !"PIE Level", i32 2}
+!3 = !{i32 7, !"uwtable", i32 2}
+!4 = !{i32 7, !"frame-pointer", i32 2}
+!5 = !{i32 1, !"ThinLTO", i32 0}
+!6 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+!7 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git 2288ebbec148ce8000fc2c1f63a4033cd847669e)"}
+
+^0 = module: (path: "main-LTO.bc", hash: (0, 0, 0, 0, 0))
+^1 = gv: (name: "a") ; guid = 12157170054180749580
+^2 = gv: (name: "main", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 1, live: 0, dsoLocal: 1, canAutoHide: 0), insts: 4, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 1, alwaysInline: 0, noUnwind: 1, mayThrow: 0, hasUnknownCall: 0, mustBeUnreachable: 0), calls: ((callee: ^1))))) ; guid = 15822663052811949562
+^3 = flags: 8
+^4 = blockcount: 1

llvm/test/tools/gold/X86/fatlto/fatlto.test

+;; Basic FatLTO tests.
+
+;; Clean up and initialize test dir
+; RUN: rm -rf %t
+; RUN: mkdir -p %t
+
+;; Ensure that input files contain .llvm.lto section
+; RUN: llc %p/Inputs/a-LTO.ll --filetype=obj -o %t/a-fatLTO.o
+; RUN: llvm-as %p/Inputs/a-LTO.ll -o %t/a-fatLTO.bc
+; RUN: llvm-objcopy --add-section=.llvm.lto=%t/a-fatLTO.bc %t/a-fatLTO.o
+; RUN: llvm-objcopy --set-section-flags=.llvm.lto=exclude %t/a-fatLTO.o
+; RUN: llvm-readobj -S %t/a-fatLTO.o | FileCheck --check-prefix=CHECK-A %s
+
+; CHECK-A: Name: .llvm.lto
+
+; RUN: llc %p/Inputs/main-LTO.ll --filetype=obj -o %t/main-fatLTO.o
+; RUN: llvm-as %p/Inputs/main-LTO.ll -o %t/main-fatLTO.bc
+; RUN: llvm-objcopy --add-section=.llvm.lto=%t/main-fatLTO.bc %t/main-fatLTO.o
+; RUN: llvm-objcopy --set-section-flags=.llvm.lto=exclude %t/main-fatLTO.o
+; RUN: llvm-readobj -S %t/main-fatLTO.o | FileCheck --check-prefix=CHECK-MAIN %s
+
+; CHECK-MAIN: Name: .llvm.lto
+
+; Final executable should not have .llvm.lto section no matter what the target is
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -o %t/foo-fatLTO %t/a-fatLTO.o %t/main-fatLTO.o
+; RUN: llvm-readobj -S %t/foo-fatLTO | FileCheck --check-prefix=CHECK-LTO-TARGET %s
+
+;; Check if .llvm.lto section gets aggregated in LTO target
+; CHECK-LTO-TARGET-NOT: Name: .llvm.lto
+
+;; Final executable should not have .llvm.lto section no matter what the target is
+; RUN: %gold -o %t/foo-fatNoLTO %t/a-fatLTO.o %/t/main-fatLTO.o
+; RUN: llvm-readobj -S %t/foo-fatNoLTO | FileCheck --check-prefix=CHECK-NON-LTO-TARGET %s
+
+;; Check if .llvm.lto section gets aggregated in non-LTO target
+; CHECK-NON-LTO-TARGET-NOT: Name: .llvm.lto
+
+;; Check if the LTO target executable produced from FatLTO object file is
+;; identical to the one produced from LTO modules
+; RUN: llvm-as %p/Inputs/a-LTO.ll -o %t/a-LTO.bc
+; RUN: llvm-as %p/Inputs/main-LTO.ll -o %t/main-LTO.bc
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -o %t/foo-LTO %t/a-LTO.bc %t/main-LTO.bc
+; RUN: obj2yaml %t/foo-fatLTO > %t/foo-fatLTO.yaml
+; RUN: obj2yaml %t/foo-LTO > %t/foo-LTO.yaml
+; RUN: diff %t/foo-fatLTO.yaml %t/foo-LTO.yaml
+
+;; Check if the no-LTO target executable produced from FatLTO object file is
+;; identical to the one produced from regular object files
+
+; RUN: llc %p/Inputs/a-LTO.ll --filetype=obj -o %t/a.o
+; RUN: llc %p/Inputs/main-LTO.ll --filetype=obj -o %t/main.o
+
+; RUN: %gold -o %t/foo-noLTO %t/a.o %t/main.o
+; RUN: obj2yaml %t/foo-fatNoLTO > %t/foo-fatNoLTO.yaml
+; RUN: obj2yaml %t/foo-noLTO > %t/foo-noLTO.yaml
+; RUN: diff %t/foo-fatNoLTO.yaml %t/foo-noLTO.yaml

llvm/tools/gold/gold-plugin.cpp

if (get_view) {
     if (std::error_code EC = BufferOrErr.getError()) {
       message(LDPL_ERROR, EC.message().c_str());
       return LDPS_ERR;
     }
     Buffer = std::move(BufferOrErr.get());
     BufferRef = Buffer->getMemBufferRef();
   }
 
-  // Only use bitcode files for LTO.  InputFile::create() will load bitcode
-  // from the .llvmbc section within a binary object, this bitcode is typically
-  // generated by -fembed-bitcode and is not to be used by LLVMgold.so for LTO.
-  if (identify_magic(BufferRef.getBuffer()) != file_magic::bitcode) {
-    *claimed = 0;
-    return LDPS_OK;
-  }
-
   *claimed = 1;
 
   Expected<std::unique_ptr<InputFile>> ObjOrErr = InputFile::create(BufferRef);
   if (!ObjOrErr) {
     handleAllErrors(ObjOrErr.takeError(), [&](const ErrorInfoBase &EI) {
       std::error_code EC = EI.convertToErrorCode();
       if (EC == object::object_error::invalid_file_type ||
           EC == object::object_error::bitcode_section_not_found)