Skip to content

Commit 50d368a

Browse files
authored
[LinkerWrapper] Relax ordering of static libraries for offloading (#87532)
Summary: The linker wrapper attempts to maintain consistent semantics with existing host invocations. Static libraries by default only extract if there are non-weak symbols that remain undefined. However, we have situations between linkers that put different meanings on ordering. The ld.bfd linker requires static libraries to be defined after the symbols, while `ld.lld` relaxes this rule. The linker wrapper went with the former as it's the easier solution, however this has caused a lot of issues as I've had to explain this rule to several people, it also make it difficult to include things like `libc` in the OpenMP runtime because it would sometimes be linked before or after. This patch reworks the logic to more or less perform the following logic for static libraries. 1. Split library / object inputs. 2. Include every object input and record its undefined symbols 3. Repeatedly try to extract static libraries to resolve these symbols. If a file is extracted we need to check every library again to resolve any new undefined symbols. This allows the following to work and will cause fewer issues when replacing HIP, which does `--whole-archive` so it's very likely the old logic will regress. ```console $ clang -lfoo main.c -fopenmp --offload-arch=native ```
1 parent 938a734 commit 50d368a

File tree

2 files changed

+83
-42
lines changed

2 files changed

+83
-42
lines changed

clang/test/Driver/linker-wrapper-libs.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ int bar() { return weak; }
4444
// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o -fembed-offload-object=%t.out
4545
// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
4646
// RUN: --linker-path=/usr/bin/ld %t.o %t.a -o a.out 2>&1 \
47+
// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
48+
// RUN: --linker-path=/usr/bin/ld %t.a %t.o -o a.out 2>&1 \
4749
// RUN: | FileCheck %s --check-prefix=LIBRARY-RESOLVES
4850

4951
// LIBRARY-RESOLVES: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx1030 {{.*}}.o {{.*}}.o
@@ -66,6 +68,8 @@ int bar() { return weak; }
6668
// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o -fembed-offload-object=%t.out
6769
// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
6870
// RUN: --linker-path=/usr/bin/ld %t.o %t.a -o a.out 2>&1 \
71+
// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
72+
// RUN: --linker-path=/usr/bin/ld %t.a %t.o -o a.out 2>&1 \
6973
// RUN: | FileCheck %s --check-prefix=LIBRARY-GLOBAL
7074

7175
// LIBRARY-GLOBAL: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx1030 {{.*}}.o {{.*}}.o

clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Lines changed: 79 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,9 +1448,9 @@ getDeviceInput(const ArgList &Args) {
14481448
StringSaver Saver(Alloc);
14491449

14501450
// Try to extract device code from the linker input files.
1451-
DenseMap<OffloadFile::TargetID, SmallVector<OffloadFile>> InputFiles;
1452-
DenseMap<OffloadFile::TargetID, DenseMap<StringRef, Symbol>> Syms;
14531451
bool WholeArchive = Args.hasArg(OPT_wholearchive_flag) ? true : false;
1452+
SmallVector<OffloadFile> ObjectFilesToExtract;
1453+
SmallVector<OffloadFile> ArchiveFilesToExtract;
14541454
for (const opt::Arg *Arg : Args.filtered(
14551455
OPT_INPUT, OPT_library, OPT_whole_archive, OPT_no_whole_archive)) {
14561456
if (Arg->getOption().matches(OPT_whole_archive) ||
@@ -1486,50 +1486,87 @@ getDeviceInput(const ArgList &Args) {
14861486
if (Error Err = extractOffloadBinaries(Buffer, Binaries))
14871487
return std::move(Err);
14881488

1489-
// We only extract archive members that are needed.
1490-
bool IsArchive = identify_magic(Buffer.getBuffer()) == file_magic::archive;
1491-
bool Extracted = true;
1492-
while (Extracted) {
1493-
Extracted = false;
1494-
for (OffloadFile &Binary : Binaries) {
1495-
// If the binary was previously extracted it will be set to null.
1496-
if (!Binary.getBinary())
1489+
for (auto &OffloadFile : Binaries) {
1490+
if (identify_magic(Buffer.getBuffer()) == file_magic::archive &&
1491+
!WholeArchive)
1492+
ArchiveFilesToExtract.emplace_back(std::move(OffloadFile));
1493+
else
1494+
ObjectFilesToExtract.emplace_back(std::move(OffloadFile));
1495+
}
1496+
}
1497+
1498+
// Link all standard input files and update the list of symbols.
1499+
DenseMap<OffloadFile::TargetID, SmallVector<OffloadFile>> InputFiles;
1500+
DenseMap<OffloadFile::TargetID, DenseMap<StringRef, Symbol>> Syms;
1501+
for (OffloadFile &Binary : ObjectFilesToExtract) {
1502+
if (!Binary.getBinary())
1503+
continue;
1504+
1505+
SmallVector<OffloadFile::TargetID> CompatibleTargets = {Binary};
1506+
for (const auto &[ID, Input] : InputFiles)
1507+
if (object::areTargetsCompatible(Binary, ID))
1508+
CompatibleTargets.emplace_back(ID);
1509+
1510+
for (const auto &[Index, ID] : llvm::enumerate(CompatibleTargets)) {
1511+
Expected<bool> ExtractOrErr = getSymbols(
1512+
Binary.getBinary()->getImage(), Binary.getBinary()->getOffloadKind(),
1513+
/*IsArchive=*/false, Saver, Syms[ID]);
1514+
if (!ExtractOrErr)
1515+
return ExtractOrErr.takeError();
1516+
1517+
// If another target needs this binary it must be copied instead.
1518+
if (Index == CompatibleTargets.size() - 1)
1519+
InputFiles[ID].emplace_back(std::move(Binary));
1520+
else
1521+
InputFiles[ID].emplace_back(Binary.copy());
1522+
}
1523+
}
1524+
1525+
// Archive members only extract if they define needed symbols. We do this
1526+
// after every regular input file so that libraries may be included out of
1527+
// order. This follows 'ld.lld' semantics which are more lenient.
1528+
bool Extracted = true;
1529+
while (Extracted) {
1530+
Extracted = false;
1531+
for (OffloadFile &Binary : ArchiveFilesToExtract) {
1532+
// If the binary was previously extracted it will be set to null.
1533+
if (!Binary.getBinary())
1534+
continue;
1535+
1536+
SmallVector<OffloadFile::TargetID> CompatibleTargets = {Binary};
1537+
for (const auto &[ID, Input] : InputFiles)
1538+
if (object::areTargetsCompatible(Binary, ID))
1539+
CompatibleTargets.emplace_back(ID);
1540+
1541+
for (const auto &[Index, ID] : llvm::enumerate(CompatibleTargets)) {
1542+
// Only extract an if we have an an object matching this target.
1543+
if (!InputFiles.count(ID))
14971544
continue;
14981545

1499-
SmallVector<OffloadFile::TargetID> CompatibleTargets = {Binary};
1500-
for (const auto &[ID, Input] : InputFiles)
1501-
if (object::areTargetsCompatible(Binary, ID))
1502-
CompatibleTargets.emplace_back(ID);
1503-
1504-
for (const auto &[Index, ID] : llvm::enumerate(CompatibleTargets)) {
1505-
// Only extract an if we have an an object matching this target.
1506-
if (IsArchive && !WholeArchive && !InputFiles.count(ID))
1507-
continue;
1508-
1509-
Expected<bool> ExtractOrErr = getSymbols(
1510-
Binary.getBinary()->getImage(),
1511-
Binary.getBinary()->getOffloadKind(), IsArchive, Saver, Syms[ID]);
1512-
if (!ExtractOrErr)
1513-
return ExtractOrErr.takeError();
1514-
1515-
Extracted = !WholeArchive && *ExtractOrErr;
1516-
1517-
// Skip including the file if it is an archive that does not resolve
1518-
// any symbols.
1519-
if (IsArchive && !WholeArchive && !Extracted)
1520-
continue;
1521-
1522-
// If another target needs this binary it must be copied instead.
1523-
if (Index == CompatibleTargets.size() - 1)
1524-
InputFiles[ID].emplace_back(std::move(Binary));
1525-
else
1526-
InputFiles[ID].emplace_back(Binary.copy());
1527-
}
1546+
Expected<bool> ExtractOrErr =
1547+
getSymbols(Binary.getBinary()->getImage(),
1548+
Binary.getBinary()->getOffloadKind(), /*IsArchive=*/true,
1549+
Saver, Syms[ID]);
1550+
if (!ExtractOrErr)
1551+
return ExtractOrErr.takeError();
15281552

1529-
// If we extracted any files we need to check all the symbols again.
1530-
if (Extracted)
1531-
break;
1553+
Extracted = *ExtractOrErr;
1554+
1555+
// Skip including the file if it is an archive that does not resolve
1556+
// any symbols.
1557+
if (!Extracted)
1558+
continue;
1559+
1560+
// If another target needs this binary it must be copied instead.
1561+
if (Index == CompatibleTargets.size() - 1)
1562+
InputFiles[ID].emplace_back(std::move(Binary));
1563+
else
1564+
InputFiles[ID].emplace_back(Binary.copy());
15321565
}
1566+
1567+
// If we extracted any files we need to check all the symbols again.
1568+
if (Extracted)
1569+
break;
15331570
}
15341571
}
15351572

0 commit comments

Comments
 (0)