Reland [llvm] Preliminary fat-lto-objects support

[paulkirth]

Summary

Fat LTO objects contain both LTO compatible IR, as well as generated
object code. This allows users to defer the choice of whether to use LTO
or not to link-time. This is a feature available in GCC for some time,
and makes the existing -ffat-lto-objects flag functional in the same
way as GCC's.

Within LLVM, we add a new EmbedBitcodePass that serializes the module to
the object file, and expose a new pass pipeline for compiling fat
objects. The new pipeline initially clones the module and runs the
selected (Thin)LTOPrelink pipeline, after which it will serialize the
module into a .llvm.lto section of an ELF file. When compiling for
(Thin)LTO, this normally the point at which the compiler would emit a
object file containing the bitcode and metadata.

After that point we compile the original module using the
PerModuleDefaultPipeline used for non-LTO compilation. We generate
standard object files at the end of this pipeline, which contain machine
code and the new .llvm.lto section containing bitcode.

Since the two pipelines operate on different copies of the module, we
can be sure that the bitcode in the .llvm.lto section and object code
in .text are congruent with the existing output produced by the
default and LTO pipelines.

Original RFC: https://discourse.llvm.org/t/rfc-ffat-lto-objects-support/63977

Earlier versions of this patch were missing REQUIRES lines for llc
related tests in Transforms/EmbedBitcode. Those tests are now under
CodeGen/X86, which should avoid running the check on unsupported
platforms.

The EmbedbBitcodePass also returned PreservedAnalyses::all when adding a
metadata section, which failed expensive checks, since it modified the
module. This is now corrected.

[paulkirth]

llvm/lib/Passes/PassBuilderPipelines.cpp

1466

I don't think the prior code mimic'ed the non-LTO pipeline, however. My understanding is that the prior version did the following for the non embedded code on which it would generate a native object:

(Thin)LTOPreLink + ModuleSimplification(ThinOrFullLTOPhase::None) + ModuleOptimization(ThinOrFullLTOPhase::None)

So you would have been essentially duplicating ModuleSimplification (since it is invoked in both the *LTO pre-link and again with LTO Phase = None). That can cause some issues because there are passes that are only meant to be invoked once that you could duplicate. A good example is PGO instrumentation/annotation (see the conditions under which addPGOInstrPasses is called from buildModuleSimplificationPipeline).

With your new version, my understanding is that you are calling:

1. buildPerModuleDefaultPipeline on original Module -> generate native object
2. (Thin)LTOPreLink on Module clone -> embed IR

And buildPerModuleDefaultPipeline is largely just ModuleSimplification + ModuleOptimization. So I'm not sure about why it would be more wasteful (other than cloning the Module) - you are not running more pipelines total than in the prior code. Unless I am misunderstanding the former or current code, which is possible!

I think that's an accurate summary. While my intent originally was to closely mimic the default pipeline, I don't think it was very close in reality. Also, as you pointed out the new approach isn't nearly as wasteful as I felt initially.

[aeubanks]

In D146776#4302238, @tejohnson wrote:

I agree ensuring full performance with FatLTO's non-LTO native object isn't terribly important. But my concern is whether this results in some unexpected behavior with certain passes not being run at all - e.g. ICP won't run at all if you are doing SamplePGO in this configuration.

...

As mentioned earlier, I think the question is whether it will be a surprise when using SamplePGO that certain things simply don't get run when using the non-LTO native objects. I don't know enough about when these will get used to know if this is a configuration anyone will care about. If it is, I suppose one approach would be to invoke the longer pipelines only under SamplePGO. Either way, this probably needs some clear documentation.

These are all performance concerns for the non-LTO native objects, not correctness right? I think if we clearly document that performance might be very meh for fat LTO non-LTO native objects this would be fine.

llvm/include/llvm/Bitcode/EmbedBitcodePass.h

33

is this default param necessary?

llvm/lib/Passes/PassBuilderPipelines.cpp

1235

do we still need IsFatLTO with the latest approach?

1466

IIUC the previous version was ThinLTOPreLink + ModuleOptimization(ThinOrFullLTOPhase::None), where we'd write out the module after pre-link and before module optimization to some global. So we'd avoid running the simplification pipeline twice with that. (the problem is that the simplification pipeline is slightly different between a normal build and a ThinLTO build)

the current version will run the simplification pipeline twice now, once for ThinLTO and once for the default pipeline

llvm/lib/Passes/PassRegistry.def

61

these should be parameterizable, see MODULE_PASS_WITH_PARAMS

[paulkirth]

In D146776#4374005, @aeubanks wrote:

These are all performance concerns for the non-LTO native objects, not correctness right? I think if we clearly document that performance might be very meh for fat LTO non-LTO native objects this would be fine.

In the prior version, I think its too hard to say if running pipelines, like ModuleSimplification, multiple times is safe/correct without some fairly detailed analysis. I can easily imagine any transform that is only expected to run once may introduce a subtle bug if run again on the same module. Nothing obvious here jumps out at me, but I'm also not confident enough to say that it can't happen.

That said, since FatLTO no longer use a modified/custom pipeline but uses the (Thin)LTO and Module piplines on independed modules, there should no longer be any correctness nor performance issues over a normal compilation for the non-LTO object code.

llvm/include/llvm/Bitcode/EmbedBitcodePass.h

33

IIRC it was required for some reason, but let me double check that.

llvm/lib/Passes/PassBuilderPipelines.cpp

1235

That's a good point. I don't think so, so I can simplify this futher. Thanks for pointing this out.

llvm/lib/Passes/PassRegistry.def

61

That is a much better way to handle this. Thank you.

[paulkirth]

Update summary, documentation, and address comments.

[aeubanks]

llvm/include/llvm/Bitcode/EmbedBitcodePass.h

27

extraneous ;

29

this is never used

44

if you really wanted to hook this up to the pipeline parsing (so you could specify a sub-passmanager) you'd have to thread through all the parsing code. probably not worth it

llvm/include/llvm/Passes/PassBuilder.h

247

comment out of date

llvm/lib/Passes/PassBuilderPipelines.cpp

1235

still not removed?

1452

this comment is obsolete now

[paulkirth]

Address remaining comments

• Fix Doc string
• Remove obsolete comment
• Remove uses of IsThinkLTO
• Remove some dead code
• Fix some typos
• Fix test looking for wrong pass name in the error string
• git clang-format

llvm/lib/Passes/PassBuilderPipelines.cpp

1452

I was thinking once that patch lands we can take the ThinOrFullLTOPhase direcly and avoid the branch, but maybe it still makes sense to leave this as is.

[aeubanks]

this basically lgtm

llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp

31–38

these should be unnecessary with the newly added pass params, e.g. -passes=embed-bitcode<emit-summary>

[paulkirth]

Rebase

• Fix typo in docs
• Remove unneeded cl::opts

[paulkirth]

@tejohnson @nikic any lingering concerns?

[tejohnson]

LGTM with a couple of minor nits and a question back to @nikic about one of this comments.

llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp

31

true seems to be the default, did you mean false? The same could probably be said for the later call to report_fatal_error (ELF format). I actually think this one seems less like a user error and more like a compiler error (if this pass gets run twice somehow).

50

Document constant parameter (nullptr)

llvm/test/Bitcode/embed.ll

16

s/sue/sure

[paulkirth]

Thanks for the feedback @tejohnson. I'm about to take off for the weekend, but I'll address these on Monday.

llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp

31

Oh, yeah, probably. I do see your point though. I guess maybe user error encompasses adding a pass multiple times?

IMO they're all kind of hard errors. I'm happy to do this either way, but if we want to suppress this one, then you're right that it should be false here.

50

ah, good catch. Thanks.

llvm/test/Bitcode/embed.ll

16

ty

[nikic]

I'm okay with the new approach -- I think it makes the feature somewhat less compelling, but it's the path of least resistance for now. However, I'd like to have it documented that we're explicitly not guaranteeing that the results are bit-for-bit identical, and this is just an artifact of the current implementation.

Two notes on the larger feature:

• I'm concerned about the lld-only limitation. Unless there is some technical limitation here, this should be supported in LLVMgold as well.
• It looks like this requires passing -fat-lto-objects to lld to actually get LTO -- and it looks like the clang doesn't do this itself even if -ffat-lto-objects is passed to the driver? Can you please check how these flags work in GCC? I'm not entirely confident on this, but I expect that if you have fat objects they will use LTO by default unless you pass -fno-lto to the driver. We should try to match whatever the GCC behavior is here.

llvm/docs/FatLTOSupport.rst

28 ↗

(On Diff #529076)

29 ↗

(On Diff #529076)

34 ↗

(On Diff #529076)

38–40 ↗

(On Diff #529076)

43 ↗

(On Diff #529076)

The pre-link pipeline is run as part of EmbedBitcode, not before it.

50 ↗

(On Diff #529076)

This seems like the important part for end users, so maybe put that in the introduction.

60 ↗

(On Diff #529076)

This is a very annoying limitation -- is it particularly hard to implementation support for this in the LLVMgold plugin?

We won't be able to use fat objects if it's only supported by LLD.

llvm/docs/ReleaseNotes.rst

85 ↗

(On Diff #529076)

Maybe link the docs instead / as well here?

llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp

31

Sorry, I meant false here.

My general rule of thumb is that if you can write a test for it, then it should not crash.

[tejohnson]

In D146776#4410967, @nikic wrote:

I'm okay with the new approach -- I think it makes the feature somewhat less compelling, but it's the path of least resistance for now. However, I'd like to have it documented that we're explicitly not guaranteeing that the results are bit-for-bit identical, and this is just an artifact of the current implementation.

Two notes on the larger feature:

• I'm concerned about the lld-only limitation. Unless there is some technical limitation here, this should be supported in LLVMgold as well.

Great point. I see that the linker changes aren't in this patch, but in another related one. I agree it should be supported by the LLVM gold plugin as well, which should be straightforward. This can be another follow on patch. It looks like you would want to modify this change made to ignore the -fembed-bitcode sections so that we do in fact claim the fat object files:
https://github.com/llvm/llvm-project/blob/main/llvm/tools/gold/gold-plugin.cpp#L541-L547

• It looks like this requires passing -fat-lto-objects to lld to actually get LTO -- and it looks like the clang doesn't do this itself even if -ffat-lto-objects is passed to the driver? Can you please check how these flags work in GCC? I'm not entirely confident on this, but I expect that if you have fat objects they will use LTO by default unless you pass -fno-lto to the driver. We should try to match whatever the GCC behavior is here.

agree

[paulkirth]

Rebase and address comments

[paulkirth]

llvm/test/Bitcode/embed-multiple.ll

2

looks like I failed to update the tests after making the errors non-fatal.

[paulkirth]

Rebase and fix test cases.

[nikic]

LGTM

llvm/docs/FatLTOSupport.rst

37 ↗

(On Diff #532211)

unmodified

60 ↗

(On Diff #532211)

I feel like this information has been repeated (with minor variations) three times: In the bullet points above, the paragraph following it, and then here...

73 ↗

(On Diff #532211)

Despite the name, the gold plugin is also supported by the bfd linker, so you actually support all common linkers (on ELF based systems).

[MaskRay]

Apologies for not commenting soon. I'll take a close look shortly.

[paulkirth]

In D146776#4429120, @MaskRay wrote:

Apologies for not commenting soon. I'll take a close look shortly.

TY!

llvm/docs/FatLTOSupport.rst

60 ↗

(On Diff #532211)

Yeah, it's definitely a bit redundant. I'll go through this one more time and try to streamline the text.

73 ↗

(On Diff #532211)

Ah, right. I completely forgot bfd could use the plugin.

[paulkirth]

Revise text in documentation.

[paulkirth]

Document constant parameter.

[paulkirth]

llvm/docs/FatLTOSupport.rst

60 ↗

(On Diff #532211)

I feel like the current version reads a bit better. WDYT?

[nikic]

llvm/docs/FatLTOSupport.rst

49 ↗

(On Diff #532271)

61 ↗

(On Diff #532271)

60 ↗

(On Diff #532211)

Looks good!

[MaskRay]

Started looking...

-DBUILD_SHARED_LIBS=on uses -Wl,-z,defs and can detect some library layering (https://llvm.org/docs/CodingStandards.html#library-layering https://maskray.me/blog/2021-06-13-dependency-related-linker-options) problems. ninja LLVMBitWriter doesn't build due to missing dependencies.

One issue can be addressed with the following change but the dependency may be a bit weird.

diff --git i/llvm/lib/Bitcode/Writer/CMakeLists.txt w/llvm/lib/Bitcode/Writer/CMakeLists.txt
index 2b17aa912016..51849d2d0fc0 100644
--- i/llvm/lib/Bitcode/Writer/CMakeLists.txt
+++ w/llvm/lib/Bitcode/Writer/CMakeLists.txt
@@ -17,2 +17,4 @@ add_llvm_component_library(LLVMBitWriter
   TargetParser
+  TransformUtils
   )

The other issue (error: undefined symbol: llvm::ThinLTOBitcodeWriterPass::run) is more tricky: LLVMipo (llvm/lib/Transforms/IPO/CMakeLists.txt) depends on LLVMBitWriter, so LLVMBitWriter cannot depend on LLVMipo.

llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp

25

Including llvm/Transforms headers seems problematic.

[MaskRay]

llvm/docs/FatLTOSupport.rst

1 ↗

(On Diff #532271)

Maybe just FatLTO.rst. For other features, we don't use the Support suffix.

18 ↗

(On Diff #532271)

llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp

33

Nit: diagnostics do not need capitalization (https://llvm.org/docs/CodingStandards.html#error-and-warning-messages) but report_fatal_error messages often do capitalization, so I think capitalization is fine, but the period is often omitted.

56

You can delete ModuleData and even Buf.

llvm/lib/Passes/PassRegistry.def

176

llvm/test/Bitcode/embed.ll

10

Nit: complete sentences in comments end with a period.

[MaskRay]

I think we need a new section type (say, SHT_LLVM_LTO) in llvm/include/llvm/BinaryFormat/ELF.h. Then add a test llvm/test/MC/ELF to use llvm-readobj -S to test this new section type.

[MaskRay]

.llvm.lto should be added beside .llvmcmd in llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp so that .llvm.lto has the SHF_EXCLUDE flag on ELF.
We need a llvm/test/CodeGen/ test using llvm-readelf -S to test the section flags (E).

[paulkirth]

In D146776#4430608, @MaskRay wrote:

Started looking...

-DBUILD_SHARED_LIBS=on uses -Wl,-z,defs and can detect some library layering (https://llvm.org/docs/CodingStandards.html#library-layering https://maskray.me/blog/2021-06-13-dependency-related-linker-options) problems. ninja LLVMBitWriter doesn't build due to missing dependencies.

One issue can be addressed with the following change but the dependency may be a bit weird.

diff --git i/llvm/lib/Bitcode/Writer/CMakeLists.txt w/llvm/lib/Bitcode/Writer/CMakeLists.txt
index 2b17aa912016..51849d2d0fc0 100644
--- i/llvm/lib/Bitcode/Writer/CMakeLists.txt
+++ w/llvm/lib/Bitcode/Writer/CMakeLists.txt
@@ -17,2 +17,4 @@ add_llvm_component_library(LLVMBitWriter
   TargetParser
+  TransformUtils
   )

The other issue (error: undefined symbol: llvm::ThinLTOBitcodeWriterPass::run) is more tricky: LLVMipo (llvm/lib/Transforms/IPO/CMakeLists.txt) depends on LLVMBitWriter, so LLVMBitWriter cannot depend on LLVMipo.

What if we move the pass into transforms? Yes, its fairly correlated to bitcode writing, but the pass is really just creating a new section using existing utilities, which could just be considered a transform. There's also an argument that this pass should just live wherever the ThinLTOBitcodeWritterPass does. I think that should fix both the issues you've mentioned here.

Do you think that would be a better solution?

[paulkirth]

Address comments about documentation.

I still need to address more of @MaskRay's comments.

[aeubanks]

In D146776#4435430, @paulkirth wrote:

In D146776#4430608, @MaskRay wrote:

Started looking...

-DBUILD_SHARED_LIBS=on uses -Wl,-z,defs and can detect some library layering (https://llvm.org/docs/CodingStandards.html#library-layering https://maskray.me/blog/2021-06-13-dependency-related-linker-options) problems. ninja LLVMBitWriter doesn't build due to missing dependencies.

One issue can be addressed with the following change but the dependency may be a bit weird.

diff --git i/llvm/lib/Bitcode/Writer/CMakeLists.txt w/llvm/lib/Bitcode/Writer/CMakeLists.txt
index 2b17aa912016..51849d2d0fc0 100644
--- i/llvm/lib/Bitcode/Writer/CMakeLists.txt
+++ w/llvm/lib/Bitcode/Writer/CMakeLists.txt
@@ -17,2 +17,4 @@ add_llvm_component_library(LLVMBitWriter
   TargetParser
+  TransformUtils
   )

The other issue (error: undefined symbol: llvm::ThinLTOBitcodeWriterPass::run) is more tricky: LLVMipo (llvm/lib/Transforms/IPO/CMakeLists.txt) depends on LLVMBitWriter, so LLVMBitWriter cannot depend on LLVMipo.

What if we move the pass into transforms? Yes, its fairly correlated to bitcode writing, but the pass is really just creating a new section using existing utilities, which could just be considered a transform. There's also an argument that this pass should just live wherever the ThinLTOBitcodeWritterPass does. I think that should fix both the issues you've mentioned here.

Do you think that would be a better solution?

yes living alongside ThinLTOBitcodeWritterPass makes sense

[paulkirth]

Address some comments

• Move EmbedBitcodePass into Transforms/IPO
• Fix misc typos + formatting
• Simplify some code in EmbedBitcodePass

I have not addressed anything related to the SHT_LLVM_LTO flag or handling of .llvm.lto yet.

[paulkirth]

llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp

54–57 ↗

(On Diff #532989)

Looks like I forgot to remove these lines.

[paulkirth]

llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp

58 ↗

(On Diff #532989)

Also, while I'm not arguing that we shouldn't add SHF_LLVM_LTO, embedBufferInModule does mark it as MD_exclude https://github.com/llvm/llvm-project/blob/f1a040298381cdfc0657d5ecba231ebe6bbef61a/llvm/lib/Transforms/Utils/ModuleUtils.cpp#L375, which will eventually become SHF_EXCLUDE.

[MaskRay]

Thanks for the update. This fixes -DBUILD_SHARED_LIBS=on build of mine.

Perhaps llvm/test/Bitcode/ tests (which don't normally test -passes=) should be moved to elsewhere, e.g. llvm/test/Transforms/EmbedBitcodePass/ ?

Another strange thing to fix:

llc a.ll => .type .Lllvm.embedded.object,@object

We don't normally set attributes on temporary symbols (.L), which do not normally have symbol table entries anyway.

llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp

9 ↗

(On Diff #532989)

If EmbedBitcodePass.h provides a descriptive comment, this comment can probably be removed. It does not give additional information to users:)

42 ↗

(On Diff #532989)

delete unneeded blank line

47 ↗

(On Diff #532989)

delete unneeded blank line

[MaskRay]

In D146776#4435885, @MaskRay wrote:

Thanks for the update. This fixes -DBUILD_SHARED_LIBS=on build of mine.

Perhaps llvm/test/Bitcode/ tests (which don't normally test -passes=) should be moved to elsewhere, e.g. llvm/test/Transforms/EmbedBitcodePass/ ?

Another strange thing to fix:

llc a.ll => .type .Lllvm.embedded.object,@object

We don't normally set attributes on temporary symbols (.L), which do not normally have symbol table entries anyway.

This is due to private but I think this is acceptable. Both GNU assembler and LLVM integrated assembler do not create a symbol table entry, so this is fine. This likely does not justify adding a special case for private lowering to the underlying ELF section.

Feel free to land this one with a readelf -S test to check SHF_EXCLUDE. Maybe give some time for the Clang patch. Landing this one shall make reviewing the Clang part easier.

llvm/docs/FatLTO.rst

62 ↗

(On Diff #532989)

LLVMgold.so plugin.

Consider using hypertext to link this to llvm/docs/GoldPlugin.rst. I don't recall immediately the reST syntax...

[paulkirth]

In D146776#4435885, @MaskRay wrote:

Thanks for the update. This fixes -DBUILD_SHARED_LIBS=on build of mine.

Perhaps llvm/test/Bitcode/ tests (which don't normally test -passes=) should be moved to elsewhere, e.g. llvm/test/Transforms/EmbedBitcodePass/ ?

Oh, yes. I should have moved those too. I'll take care of that in the next version

In D146776#4435915, @MaskRay wrote:

In D146776#4435885, @MaskRay wrote:

Thanks for the update. This fixes -DBUILD_SHARED_LIBS=on build of mine.

Perhaps llvm/test/Bitcode/ tests (which don't normally test -passes=) should be moved to elsewhere, e.g. llvm/test/Transforms/EmbedBitcodePass/ ?

Another strange thing to fix:

llc a.ll => .type .Lllvm.embedded.object,@object

We don't normally set attributes on temporary symbols (.L), which do not normally have symbol table entries anyway.

This is due to private but I think this is acceptable. Both GNU assembler and LLVM integrated assembler do not create a symbol table entry, so this is fine. This likely does not justify adding a special case for private lowering to the underlying ELF section.

I'm not sure I follow the conversation here. Is this an issue w/ the defaults used by embedBufferInModule()? or is the oddity just related to the symol being private?

Feel free to land this one with a readelf -S test to check SHF_EXCLUDE. Maybe give some time for the Clang patch. Landing this one shall make reviewing the Clang part easier.

Will do. Thanks for all the feedback.

llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp

9 ↗

(On Diff #532989)

noted. I'll expand the description in the header and remove this.

[paulkirth]

Address comments

• Add test for SHT_EXCLUDE
• Fix some formatting
• Improve documentation/description in header
• Update documentation links

[paulkirth]

Reopening until I can investigate the buildbot failures.

[MaskRay]

llvm/test/Transforms/EmbedBitcode/embed.ll

23 ↗

(On Diff #534016)

We normally indent continuation lines with 2 spaces

43 ↗

(On Diff #534016)

delete trailing blank line

[MaskRay]

llvm/test/Transforms/EmbedBitcode/embed.ll

1 ↗

(On Diff #534016)

llc needs REQUIRES: x86-registered-target

[MaskRay]

llvm/test/Transforms/EmbedBitcode/embed.ll

24 ↗

(On Diff #534016)

--elf-output-style=JSON --pretty-print is not normal. llvm-readelf -S output is much conciser.

[paulkirth]

Fix test code

• move llc check for SHF_EXCLUDE under CodeGen/X86
• fix formatting

[paulkirth]

Update summary and title for reland

[paulkirth]

Remove some unused headers & fix formatting of the include guards

[MaskRay]

Thanks! One nit in the test.

llvm/test/CodeGen/X86/fat-lto-section.ll

8 ↗

(On Diff #534074)

I typically just drop [Nr] and [ 4] from the output so that adding a new section will not cause the section index to shuffle and cause the test to fail.

[paulkirth]

Make test less brittle to ordering changes

[paulkirth]

This seems to have failed on expensive checks because we marked the pass as PreservedAnalyses::all(). I guess the simplest thing to do is to change that to none, but I think there should be something else we can do here to avoid invalidating analyses that aren't affected by inserting the metadata section.

[paulkirth]

llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp

51 ↗

(On Diff #534116)

Is there another option we can use here. none seems like overkill. Since this is basically the first thing to run on the module, I'm not sure if there is much point in trying to do anything smarter, but it certainly doesn't feel right.

[aeubanks]

llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp

51 ↗

(On Diff #534116)

perhaps something like https://reviews.llvm.org/D153855?

[paulkirth]

Don't preserve analysis to satisfy expensive checks

[paulkirth]

Modify the structural hash since we don't modify the module except to add a
metadata section.

[paulkirth]

llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp

51 ↗

(On Diff #534116)

That seems like it will work. Thanks for the suggestion. We can go back to none if we're unhappy about amending the structural hash.

[aeubanks]

llvm/lib/IR/StructuralHash.cpp

62 ↗

(On Diff #535511)

maybe we should exclude anything that begins with llvm., but that should be a separate patch, so this lgtm

[paulkirth]

Update comment.

[paulkirth]

llvm/lib/IR/StructuralHash.cpp

62 ↗

(On Diff #535511)

Filed https://github.com/llvm/llvm-project/issues/63590. I can make the follow up patch, but I may not get a chance till tomorrow.

llvm/include/llvm/Bitcode/EmbedBitcodePass.h

+//===-- EmbedBitcodePass.h - Embeds bitcode into global ---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+/// \file
+///
+/// This file provides apass which embeds the bitcode into a global variable.
+///
+//===----------------------------------------------------------------------===//
+//
+#ifndef LLVM_BITCODE_EMBEDBITCODEPASS_H
+#define LLVM_BITCODE_EMBEDBITCODEPASS_H
+
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+class Module;
+class ModulePass;
+class Pass;
+
+/// Pass embeds the current module into a global variable.
+class EmbedBitcodePass : public PassInfoMixin<EmbedBitcodePass> {
+  bool IsThinLTO;
+  bool EmitLLVMUSeLists;
+  bool EmitLTOSummary;
+
+public:
+  EmbedBitcodePass(bool IsThinLTO, bool EmitLLVMUSeLists, bool EmitLTOSummary)
+      : IsThinLTO(IsThinLTO), EmitLLVMUSeLists(EmitLLVMUSeLists),
+        EmitLTOSummary(EmitLTOSummary){};
+
+  PreservedAnalyses run(Module &M, ModuleAnalysisManager &);
+
+  static bool isRequired() { return true; }
+};
+
+} // end namespace llvm.
+
+#endif

llvm/include/llvm/Passes/PassBuilder.h

public:
   ///
   /// Note that \p Level cannot be `O0` here. The pipelines produced are
   /// only intended for use when attempting to optimize code. If frontends
   /// require some transformations for semantic reasons, they should explicitly
   /// build them.
   ModulePassManager buildPerModuleDefaultPipeline(OptimizationLevel Level,
                                                   bool LTOPreLink = false);
 
+  /// Build a fat object default optimization pipeline.
+  ///
+  /// This builds a pipeline that runs the LTO/ThinLTO pipeline, and emits a
+  /// section containing the bitcode along size the object code generated by the
+  /// PerModuleDefaultPipeline.
+  ///
+  /// Note that \p Level cannot be `O0` here. The pipelines produced are
+  /// only intended for use when attempting to optimize code. If frontends
+  /// require some transformations for semantic reasons, they should explicitly
+  /// build them.
+  ModulePassManager buildFatLTODefaultPipeline(OptimizationLevel Level,
+                                               bool ThinLTOPreLink,
+                                               bool EmitUseList,
+                                               bool EmitSummary);
+
   /// Build a pre-link, ThinLTO-targeting default optimization pipeline to
   /// a pass manager.
   ///
   /// This adds the pre-link optimizations tuned to prepare a module for
   /// a ThinLTO run. It works to minimize the IR which needs to be analyzed
   /// without making irreversible decisions which could be made better during
   /// the LTO run.
   ///
private:
   buildO1FunctionSimplificationPipeline(OptimizationLevel Level,
                                         ThinOrFullLTOPhase Phase);
 
   void addRequiredLTOPreLinkPasses(ModulePassManager &MPM);
 
   void addVectorPasses(OptimizationLevel Level, FunctionPassManager &FPM,
                        bool IsFullLTO);
 
+  void addPerModuleDefaultPipelinePasses(ModulePassManager &MPM,
+                                         OptimizationLevel Level,
+                                         bool LTOPreLink = false);
+
   static std::optional<std::vector<PipelineElement>>
   parsePipelineText(StringRef Text);
 
   Error parseModulePass(ModulePassManager &MPM, const PipelineElement &E);
   Error parseCGSCCPass(CGSCCPassManager &CGPM, const PipelineElement &E);
   Error parseFunctionPass(FunctionPassManager &FPM, const PipelineElement &E);
   Error parseLoopPass(LoopPassManager &LPM, const PipelineElement &E);
   bool parseAAPassName(AAManager &AA, StringRef Name);

llvm/lib/Bitcode/Writer/CMakeLists.txt

 add_llvm_component_library(LLVMBitWriter
   BitWriter.cpp
   BitcodeWriter.cpp
   BitcodeWriterPass.cpp
+  EmbedBitcodePass.cpp
   ValueEnumerator.cpp
 
   DEPENDS
   intrinsics_gen
 
   LINK_COMPONENTS
   Analysis
   Core
   MC
   Object
   Support
   TargetParser
   )

llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp

+//===- EmbedBitcodePass.cpp - Pass that embeds the bitcode into a global---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// EmbedBitcodePass implementation.
+//
+//===----------------------------------------------------------------------===//
+//
+#include "llvm/Bitcode/EmbedBitcodePass.h"
+
+#include "llvm/Bitcode/BitcodeWriter.h"
+#include "llvm/Bitcode/BitcodeWriterPass.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/PassManager.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Pass.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/TargetParser/Triple.h"
+#include "llvm/Transforms/IPO/ThinLTOBitcodeWriter.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
+
+using namespace llvm;
+
+PreservedAnalyses EmbedBitcodePass::run(Module &M, ModuleAnalysisManager &AM) {
+  if (M.getGlobalVariable("llvm.embedded.module", true))
+    report_fatal_error("Can only embed the module once.");
+
+  Triple T(M.getTargetTriple());
+  if (T.getObjectFormat() != Triple::ELF)
+    report_fatal_error(
+        "Embed bitcode pass currently only supports ELF object format.");
+
+  std::string Data;
+  raw_string_ostream OS(Data);
+
+  if (IsThinLTO)
+    ThinLTOBitcodeWriterPass(OS, nullptr).run(M, AM);
+  else
+    BitcodeWriterPass(OS, EmitLLVMUSeLists, EmitLTOSummary).run(M, AM);
+
+  ArrayRef<uint8_t> ModuleData((const uint8_t *)OS.str().data(),
+                               OS.str().size());
+  Constant *ModuleConstant = ConstantDataArray::get(M.getContext(), ModuleData);
+
+  llvm::GlobalVariable *EM = new llvm::GlobalVariable(
+      M, ModuleConstant->getType(), true, llvm::GlobalValue::PrivateLinkage,
+      ModuleConstant);
+
+  EM->setSection(".llvm.lto");
+  EM->setAlignment(Align(1));
+  EM->setName("llvm.embedded.module");
+  appendToCompilerUsed(M, {EM});
+
+  return PreservedAnalyses::all();
+}

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";
+    return *NameOrErr == ".llvmbc" || *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/lib/Passes/PassBuilder.cpp

 #include "llvm/Analysis/ScalarEvolutionAliasAnalysis.h"
 #include "llvm/Analysis/ScopedNoAliasAA.h"
 #include "llvm/Analysis/StackLifetime.h"
 #include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
 #include "llvm/Analysis/UniformityAnalysis.h"
+#include "llvm/Bitcode/EmbedBitcodePass.h"
 #include "llvm/CodeGen/HardwareLoops.h"
 #include "llvm/CodeGen/TypePromotion.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/PrintPasses.h"
 #include "llvm/IR/SafepointIRVerifier.h"
 #include "llvm/IR/Verifier.h"

llvm/lib/Passes/PassBuilderPipelines.cpp

 #include "llvm/Analysis/BasicAliasAnalysis.h"
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/InlineAdvisor.h"
 #include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/ScopedNoAliasAA.h"
 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
+#include "llvm/Bitcode/EmbedBitcodePass.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Passes/OptimizationLevel.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/PGOOptions.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Target/TargetMachine.h"
void PassBuilder::addVectorPasses(OptimizationLevel Level,
 
   if (IsFullLTO)
     FPM.addPass(InstCombinePass());
 }
 
 ModulePassManager
 PassBuilder::buildModuleOptimizationPipeline(OptimizationLevel Level,
                                              ThinOrFullLTOPhase LTOPhase) {
   const bool LTOPreLink = (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink ||
                            LTOPhase == ThinOrFullLTOPhase::FullLTOPreLink);
   ModulePassManager MPM;
 
   // Run partial inlining pass to partially inline functions that have
   // large bodies.
   if (RunPartialInlining)
     MPM.addPass(PartialInlinerPass());
 
if (!LTOPreLink)
     MPM.addPass(RelLookupTableConverterPass());
 
   return MPM;
 }
 
 ModulePassManager
 PassBuilder::buildPerModuleDefaultPipeline(OptimizationLevel Level,
                                            bool LTOPreLink) {
-  if (Level == OptimizationLevel::O0)
-    return buildO0DefaultPipeline(Level, LTOPreLink);
-
   ModulePassManager MPM;
+  addPerModuleDefaultPipelinePasses(MPM, Level, LTOPreLink);
+  return MPM;
+}
+
+void PassBuilder::addPerModuleDefaultPipelinePasses(ModulePassManager &MPM,
+                                                    OptimizationLevel Level,
+                                                    bool LTOPreLink) {
+  assert(Level != OptimizationLevel::O0 &&
+         "Must request optimizations for the default pipeline!");
 
   // Convert @llvm.global.annotations to !annotation metadata.
   MPM.addPass(Annotation2MetadataPass());
 
-  // Force any function attributes we want the rest of the pipeline to observe.
+  // Force any function attributes we want the rest of the pipeline to
+  // observe.
   MPM.addPass(ForceFunctionAttrsPass());
 
   // Apply module pipeline start EP callback.
   for (auto &C : PipelineStartEPCallbacks)
     C(MPM, Level);
 
   if (PGOOpt && PGOOpt->DebugInfoForProfiling)
     MPM.addPass(createModuleToFunctionPassAdaptor(AddDiscriminatorsPass()));
if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&
       PGOOpt->Action == PGOOptions::SampleUse)
     MPM.addPass(PseudoProbeUpdatePass());
 
   // Emit annotation remarks.
   addAnnotationRemarksPass(MPM);
 
   if (LTOPreLink)
     addRequiredLTOPreLinkPasses(MPM);
+}
 
+ModulePassManager
+PassBuilder::buildFatLTODefaultPipeline(OptimizationLevel Level,
+                                        bool ThinLTOPreLink, bool EmitUseList,
+                                        bool EmitSummary) {
+  assert(Level != OptimizationLevel::O0 &&
+         "Must request optimizations for the default pipeline!");
+  ModulePassManager MPM = ThinLTOPreLink
+                              ? buildThinLTOPreLinkDefaultPipeline(Level)
+                              : buildLTOPreLinkDefaultPipeline(Level);
+  MPM.addPass(EmbedBitcodePass(ThinLTOPreLink, EmitUseList, EmitSummary));
+  addPerModuleDefaultPipelinePasses(MPM, Level, /*LTOPreLink=*/false);
   return MPM;
 }
 
 ModulePassManager
 PassBuilder::buildThinLTOPreLinkDefaultPipeline(OptimizationLevel Level) {
   if (Level == OptimizationLevel::O0)
     return buildO0DefaultPipeline(Level, /*LTOPreLink*/true);
 
   ModulePassManager MPM;
 
   // Convert @llvm.global.annotations to !annotation metadata.
   MPM.addPass(Annotation2MetadataPass());

llvm/lib/Passes/PassRegistry.def

 MODULE_PASS("coro-early", CoroEarlyPass())
 MODULE_PASS("coro-cleanup", CoroCleanupPass())
 MODULE_PASS("cross-dso-cfi", CrossDSOCFIPass())
 MODULE_PASS("deadargelim", DeadArgumentEliminationPass())
 MODULE_PASS("debugify", NewPMDebugifyPass())
 MODULE_PASS("dot-callgraph", CallGraphDOTPrinterPass())
 MODULE_PASS("elim-avail-extern", EliminateAvailableExternallyPass())
 MODULE_PASS("extract-blocks", BlockExtractorPass({}, false))
+MODULE_PASS("embed-bitcode", EmbedBitcodePass(true, true, true))
 MODULE_PASS("forceattrs", ForceFunctionAttrsPass())
 MODULE_PASS("function-import", FunctionImportPass())
 MODULE_PASS("globaldce", GlobalDCEPass())
 MODULE_PASS("globalopt", GlobalOptPass())
 MODULE_PASS("globalsplit", GlobalSplitPass())
 MODULE_PASS("hotcoldsplit", HotColdSplittingPass())
 MODULE_PASS("inferattrs", InferFunctionAttrsPass())
 MODULE_PASS("inliner-wrapper", ModuleInlinerWrapperPass())
MODULE_PASS_WITH_PARAMS("ipsccp",
                         "IPSCCPPass",
                         [](IPSCCPOptions Opts) {
                           return IPSCCPPass(Opts);
                         },
                         parseIPSCCPOptions,
                         "no-func-spec;func-spec")
 #undef MODULE_PASS_WITH_PARAMS
 
 #ifndef CGSCC_ANALYSIS
 #define CGSCC_ANALYSIS(NAME, CREATE_PASS)
 #endif
 CGSCC_ANALYSIS("no-op-cgscc", NoOpCGSCCAnalysis())
 CGSCC_ANALYSIS("fam-proxy", FunctionAnalysisManagerCGSCCProxy())
 CGSCC_ANALYSIS("pass-instrumentation", PassInstrumentationAnalysis(PIC))
 #undef CGSCC_ANALYSIS
 
 #ifndef CGSCC_PASS

llvm/test/Bitcode/embed-multiple.ll

+; RUN: not --crash opt --mtriple x86_64-unknown-linux-gnu < %s -passes=embed-bitcode -S 2>&1 | FileCheck %s
+
+@a = global i32 1
[e-mail] = private constant [4 x i8] c"BC\C0\DE"
+
+; CHECK: LLVM ERROR: Can only embed the module once.

llvm/test/Bitcode/embed-unsupported-object-format.ll

+; RUN: not --crash opt --mtriple powerpc64-unknown-aix < %s -passes=embed-bitcode -S 2>&1 | FileCheck %s
+
+@a = global i32 1
+
+; CHECK: LLVM ERROR: Embed bitcode pass currently only supports ELF object format

llvm/test/Bitcode/embed.ll

+; RUN: opt --mtriple x86_64-unknown-linux-gnu < %s -passes=embed-bitcode -S | FileCheck %s
+
+@a = global i32 1
+
+; CHECK: @a = global i32 1
+; CHECK: @llvm.embedded.module = private constant {{.*}}, section ".llvm.lto", align 1
+; CHECK: @llvm.compiler.used = appending global [1 x ptr] [ptr @llvm.embedded.module], section "llvm.metadata"