Page MenuHomePhabricator

[WIP] First draft of -ffat-lto-object related changes: front-end additions, duplication and repurposing -fembed-bitcode related code.
DraftPublic

Authored by arda on Aug 3 2022, 12:23 PM.
This is a draft revision that has not yet been submitted for review.

Details

Summary

LLVM users currently have to choose at build-time whether to generate
object code or bitcode. Only having these options can be limiting since
translation units can often be shared between many targets. These different
targets may have different requirements in terms of compile time or
performance. For instance, the benefits for LTO might be worth the overhead for
main targets whereas this might not be the case for the tests.

GCC allows GIMPLE bytecode to be saved alongside object code if the
-ffat-lto-objects option is passed. This “fat” object format allows one to
build one set of fat objects which could be used for targets with different
requirements since the decision to use LTO can be made at link time.

Our goal is to implement support for emitting bitcode alongside object code in
LLVM and use that support to implement the -ffat-lto-objects option in Clang.
Doing so would bring Clang to parity with GCC. We will initially focus on ELF,
but this support could be extended to other formats later.

Diff Detail

Unit TestsFailed

TimeTest
3,680 msx64 debian > Clang.Driver::clang_f_opts.c
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -### -S -fasm -fblocks -fbuiltin -fno-math-errno -fcommon -fpascal-strings -fno-blocks -fno-builtin -fmath-errno -fno-common -fno-pascal-strings -fblocks -fbuiltin -fmath-errno -fcommon -fpascal-strings -fsplit-stack /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/clang_f_opts.c 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck -check-prefix=CHECK-OPTIONS1 /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/clang_f_opts.c
60,040 msx64 debian > MLIR.Examples/standalone::test.toy
Script: -- : 'RUN: at line 1'; /usr/bin/cmake /var/lib/buildkite-agent/builds/llvm-project/mlir/examples/standalone -G "Ninja" -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/var/lib/buildkite-agent/builds/llvm-project/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld

Event Timeline

arda created this revision.Aug 3 2022, 12:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2022, 12:23 PM
arda updated this revision to Diff 449794.Aug 3 2022, 3:05 PM

Remove the unrelated format changes.

Can we get the proposed LLD changes as well. Separate patches are probably best.

clang/lib/CodeGen/BackendUtil.cpp
196

I'd drop this, since its unrelated. If you still want to fix it, you can submit a separate change.

clang/lib/Driver/ToolChains/Clang.cpp
7120

I think these can go away, right?

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
4959

Can you make the error a bit more specific? Like let them know this is a "Fat LTO" problem? I'm not sure how well the issue will be surfaced as is.

There's been some discussion re: the use of unreachable. Not all of it applies here, especially in a prototype, but its a point to consider as work develops.

see https://discourse.llvm.org/t/llvm-unreachable-is-widely-misused/60587 for more context.

I know this is a copy paste from existing code, but a fatal error might be the better choice. What do you think?

5080

Does this just do what embed bitcode does? My understanding was that LTO generated IR is emitted into object files differently than the embed bitcode scenario. I'm basing that on the discussion at https://discourse.llvm.org/t/rfc-ffat-lto-objects-support/63977

I'd also like to see tests for this.

arda marked 2 inline comments as done.Aug 5 2022, 1:58 PM
arda added inline comments.
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
4959

From that discussion, it seems like unreachable should not be used in this case. I agree that fatal error might be a better choice here. Thanks!

5080

From my research, -fembed-bitcode was originally used for recompilation purposes, not for LTO. Since our scope is limited to ThinLTO, this prototype currently is used to embed ThinLTO bitcode into the FatLTO object as follows:

clang -c -ffat-lto-objects -o fatLTO.o -x ir thinLTO.bc
arda updated this revision to Diff 450402.Aug 5 2022, 2:10 PM

Addressed Paul's first comments.

paulkirth added inline comments.Aug 10 2022, 9:19 AM
clang/include/clang/Driver/ToolChain.h
572

This file mostly uses toolchain, so let's try to be consistent. We don't need to worry about other uses in this patch.

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
5080

Looking at this a second time, I'm not sure I follow your meaning here.

What I meant was that the way EmbedBitcode worked was not TheRightThing according to that discussion. It appears as though your change just copies its logic.

So if I want to compile a C file and get a fat-LTO object? When do the LTO passes get added to the pipeline? I don't mean the big whole program optimization stuff that gets invoked at link-time, I mean the module level passes and summary generation? It's not clear to me when those will run after invoking clang on a source file.

can you elaborate on how this is intended to work? I seem to be missing something.

phosek added inline comments.Aug 15 2022, 6:46 PM
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
4965

Since this section is LLVM specific, I think it should use an .llvm prefix, so the name could be .llvm.lto. This is similar to GCC which uses .gcc.lto_ prefixed section names.

tstellar removed a subscriber: tstellar.
arda added a comment.Aug 22 2022, 10:43 PM

The changes in this patch have been moved to here: https://reviews.llvm.org/D131618