-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[LTO][LLD] Prevent invalid LTO libfunc transforms #164916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2701,15 +2701,30 @@ static void markBuffersAsDontNeed(Ctx &ctx, bool skipLinkedOutput) { | |
| template <class ELFT> | ||
| void LinkerDriver::compileBitcodeFiles(bool skipLinkedOutput) { | ||
| llvm::TimeTraceScope timeScope("LTO"); | ||
| // Capture the triple before moving the bitcode into the bitcode compiler. | ||
| std::optional<llvm::Triple> tt; | ||
| if (!ctx.bitcodeFiles.empty()) | ||
| tt = llvm::Triple(ctx.bitcodeFiles.front()->obj->getTargetTriple()); | ||
| // Compile bitcode files and replace bitcode symbols. | ||
| lto.reset(new BitcodeCompiler(ctx)); | ||
| for (BitcodeFile *file : ctx.bitcodeFiles) | ||
| lto->add(*file); | ||
|
|
||
| if (!ctx.bitcodeFiles.empty()) | ||
| llvm::BumpPtrAllocator alloc; | ||
| llvm::StringSaver saver(alloc); | ||
| SmallVector<StringRef> bitcodeLibFuncs; | ||
| if (!ctx.bitcodeFiles.empty()) { | ||
| markBuffersAsDontNeed(ctx, skipLinkedOutput); | ||
| for (StringRef libFunc : lto::LTO::getLibFuncSymbols(*tt, saver)) { | ||
| Symbol *sym = ctx.symtab->find(libFunc); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the test for sym == nullptr be combined with the find? |
||
| if (!sym) | ||
| continue; | ||
| if (isa<BitcodeFile>(sym->file)) | ||
| bitcodeLibFuncs.push_back(libFunc); | ||
| } | ||
| } | ||
|
|
||
| ltoObjectFiles = lto->compile(); | ||
| ltoObjectFiles = lto->compile(bitcodeLibFuncs); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the callsite alone this could be interpreted as just compiling the bitcodeLibFuncs. Suggestions:
|
||
| for (auto &file : ltoObjectFiles) { | ||
| auto *obj = cast<ObjFile<ELFT>>(file.get()); | ||
| obj->parse(/*ignoreComdats=*/true); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| ; REQUIRES: x86 | ||
|
|
||
| ; RUN: rm -rf %t && split-file %s %t && cd %t | ||
| ; RUN: llvm-as main.ll -o main.o | ||
| ; RUN: llvm-as bcmp.ll -o bcmp.o | ||
| ; RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux-gnu memcmp.s -o memcmp.o | ||
| ; RUN: llvm-ar rc libc.a bcmp.o memcmp.o | ||
|
|
||
| ;; Ensure that no memcmp->bcmp translation occurs during LTO because bcmp is in | ||
| ;; bitcode, but was not brought into the link. This would fail the link by | ||
| ;; extracting bitcode after LTO. | ||
| ; RUN: ld.lld -o out main.o -L. -lc | ||
| ; RUN: llvm-nm out | FileCheck %s | ||
|
|
||
| ;--- bcmp.ll | ||
| target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128" | ||
| target triple = "x86_64-unknown-linux-gnu" | ||
|
|
||
| define i32 @bcmp(ptr %0, ptr %1, i64 %2) { | ||
| ret i32 0 | ||
| } | ||
|
|
||
| ;--- memcmp.s | ||
| .globl memcmp | ||
| memcmp: | ||
| ret | ||
|
|
||
| ;--- main.ll | ||
| target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128" | ||
| target triple = "x86_64-unknown-linux-gnu" | ||
|
|
||
| define i1 @_start(ptr %0, ptr %1, i64 %2) { | ||
| %cmp = call i32 @memcmp(ptr %0, ptr %1, i64 %2) | ||
| %eq = icmp eq i32 %cmp, 0 | ||
| ret i1 %eq | ||
| } | ||
|
|
||
| ; CHECK-NOT: bcmp | ||
| ; CHECK: memcmp | ||
| declare i32 @memcmp(ptr, ptr, i64) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are making an assumption that the triple is homogenous (modulo libcall differences) across all the bitcode files. Ideally this would be the case, but build systems being build systems there could be some differences, I'm thinking about normalised triples where
-marchmight differ.Looking at getLibFuncSymbols it seems like most if not all the variation in libcall availability is in the environment and I wouldn't expect that to vary across the build system.
I don't think we need to change anything here, but could be worth stating our assumptions in a comment.