[DTLTO][Clang] Add support for Integrated Distributed ThinLTO #147265
+117
−19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch introduces support for Integrated Distributed ThinLTO (DTLTO) in Clang. DTLTO enables the distribution of ThinLTO backend compilations via external distribution systems, such as Incredibuild, during the traditional link step: https://llvm.org/docs/DTLTO.html. Testing: - `lit` test coverage has been added to Clang's Driver tests. - The DTLTO cross-project tests will use this Clang support. For the design discussion of the DTLTO feature, see: llvm#126654 Note that I have removed the forwarding of -mllvm options to the backend compilations which was discussed in the design review from this patch. LTO configuration for DTLTO will be addressed in a follow-up patch. In the meantime -mllvm options can be forwarded manually if required.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: bd1976bris (bd1976bris) ChangesThis patch introduces support for Integrated Distributed ThinLTO (DTLTO) in Clang. DTLTO enables the distribution of ThinLTO backend compilations via external distribution systems, such as Incredibuild, during the traditional link step: https://llvm.org/docs/DTLTO.html. Testing:
For the design discussion of the DTLTO feature, see: #126654 Note that I have removed the forwarding of -mllvm options to the backend compilations which was discussed in the design review from this patch. LTO configuration for DTLTO will be addressed in a follow-up patch. In the meantime -mllvm options can be forwarded manually if required. Full diff: https://github.com/llvm/llvm-project/pull/147265.diff 5 Files Affected:
diff --git a/clang/docs/ThinLTO.rst b/clang/docs/ThinLTO.rst
index c042547678919..687795ac655a7 100644
--- a/clang/docs/ThinLTO.rst
+++ b/clang/docs/ThinLTO.rst
@@ -240,6 +240,38 @@ The ``BOOTSTRAP_LLVM_ENABLE_LTO=Thin`` will enable ThinLTO for stage 2 and
stage 3 in case the compiler used for stage 1 does not support the ThinLTO
option.
+Integrated Distributed ThinLTO (DTLTO)
+--------------------------------------
+
+Integrated Distributed ThinLTO (DTLTO) enables the distribution of backend
+ThinLTO compilations via external distribution systems, such as Incredibuild,
+during the traditional link step.
+
+The implementation is documented here: https://llvm.org/docs/DTLTO.html.
+
+DTLTO requires the LLD linker (``-fuse-ld=lld``).
+
+``-fthinlto-distributor=<path>``
+ - Specifies the ``<path>`` to the distributor process executable for DTLTO.
+ - If specified, ThinLTO backend compilations will be distributed by LLD.
+
+``-Xthinlto-distributor=<arg>``
+ - Passes ``<arg>`` to the distributor process (see ``-fthinlto-distributor=``).
+ - Can be specified multiple times to pass multiple options.
+ - Multiple options can also be specified by separating them with commas.
+
+Examples:
+ - ``clang -flto=thin -fthinlto-distributor=incredibuild.exe -Xthinlto-distributor=--verbose,--j10 -fuse-ld=lld``
+ - ``clang -flto=thin -fthinlto-distributor=$(which python) -Xthinlto-distributor=incredibuild.py -fuse-ld=lld``
+
+If ``-fthinlto-distributor=`` is specified, Clang supplies the path to a
+compiler to be executed remotely to perform the ThinLTO backend
+compilations. Currently, this is Clang itself.
+
+Note that currently, DTLTO is only supported in some LLD flavors. Support will
+be added to other LLD flavours in the future.
+See `DTLTO <https://lld.llvm.org/dtlto.html>`_ for more information.
+
More Information
================
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0c8a219b19bf4..9c6f77af97be0 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -990,6 +990,13 @@ def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
Visibility<[ClangOption, CLOption, FlangOption]>,
HelpText<"Pass <arg> to the linker">, MetaVarName<"<arg>">,
Group<Link_Group>;
+def Xthinlto_distributor_EQ : CommaJoined<["-"], "Xthinlto-distributor=">,
+ Flags<[LinkOption]>,
+ Visibility<[ClangOption, CLOption]>,
+ HelpText<"Pass <arg> to the ThinLTO distributor process. Can be specified "
+ "multiple times or with comma-separated values.">,
+ MetaVarName<"<arg>">,
+ Group<Link_Group>;
def Xoffload_linker : JoinedAndSeparate<["-"], "Xoffload-linker">,
Visibility<[ClangOption, FlangOption]>,
HelpText<"Pass <arg> to the offload linkers or the ones identified by -<triple>">,
@@ -4233,7 +4240,12 @@ def ffinite_loops: Flag<["-"], "ffinite-loops">, Group<f_Group>,
def fno_finite_loops: Flag<["-"], "fno-finite-loops">, Group<f_Group>,
HelpText<"Do not assume that any loop is finite.">,
Visibility<[ClangOption, CC1Option]>;
-
+def fthinlto_distributor_EQ : Joined<["-"], "fthinlto-distributor=">,
+ Group<f_Group>,
+ HelpText<"Path to the ThinLTO distributor process. If specified, "
+ "ThinLTO backend compilations will be distributed by LLD">,
+ MetaVarName<"<path>">,
+ Visibility<[ClangOption, CLOption]>;
def ftrigraphs : Flag<["-"], "ftrigraphs">, Group<f_Group>,
HelpText<"Process trigraph sequences">, Visibility<[ClangOption, CC1Option]>;
def fno_trigraphs : Flag<["-"], "fno-trigraphs">, Group<f_Group>,
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index f5e2655857432..3e9ca8f79d160 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -455,6 +455,21 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA,
addLTOOptions(ToolChain, Args, CmdArgs, Output, Inputs,
D.getLTOMode() == LTOK_Thin);
+ // Forward the DTLTO options to the linker. We add these unconditionally,
+ // rather than in addLTOOptions() as it is the linker that decides whether to
+ // do LTO or not dependent upon whether there are any bitcode input files in
+ // the link.
+ if (Arg *A = Args.getLastArg(options::OPT_fthinlto_distributor_EQ)) {
+ CmdArgs.push_back(
+ Args.MakeArgString("--thinlto-distributor=" + Twine(A->getValue())));
+ CmdArgs.push_back(
+ Args.MakeArgString("--thinlto-remote-compiler=" +
+ Twine(ToolChain.getDriver().getClangProgramPath())));
+
+ for (auto A : Args.getAllArgValues(options::OPT_Xthinlto_distributor_EQ))
+ CmdArgs.push_back(Args.MakeArgString("--thinlto-distributor-arg=" + A));
+ }
+
if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
CmdArgs.push_back("--no-demangle");
diff --git a/clang/test/Driver/DTLTO/dtlto.c b/clang/test/Driver/DTLTO/dtlto.c
new file mode 100644
index 0000000000000..a23a10fdfb055
--- /dev/null
+++ b/clang/test/Driver/DTLTO/dtlto.c
@@ -0,0 +1,43 @@
+// REQUIRES: lld
+
+/// Check DTLTO options are forwarded to the linker.
+
+// RUN: echo "--target=x86_64-linux-gnu \
+// RUN: -Xthinlto-distributor=distarg1 \
+// RUN: -Xthinlto-distributor=distarg2,distarg3 \
+// RUN: -fuse-ld=lld" > %t.rsp
+
+/// Check that options are forwarded as expected with --thinlto-distributor=.
+// RUN: %clang -### @%t.rsp -fthinlto-distributor=dist.exe %s 2>&1 | \
+// RUN: FileCheck %s --implicit-check-not=warning
+
+// CHECK: ld.lld
+// CHECK-SAME: "--thinlto-distributor=dist.exe"
+// CHECK-SAME: "--thinlto-remote-compiler={{.*}}clang
+// CHECK-SAME: "--thinlto-distributor-arg=distarg1"
+// CHECK-SAME: "--thinlto-distributor-arg=distarg2"
+// CHECK-SAME: "--thinlto-distributor-arg=distarg3"
+
+
+/// Check that options are not added without --thinlto-distributor= and
+/// that there is an unused option warning issued for -Xthinlto-distributor=
+/// options. We specify -flto here as these options should be unaffected by it.
+// RUN: %clang -### @%t.rsp -flto=thin %s 2>&1 | \
+// RUN: FileCheck %s --check-prefixes=NONE,NOMORE --implicit-check-not=warning
+
+// NONE: warning: argument unused during compilation: '-Xthinlto-distributor=distarg1'
+// NONE: warning: argument unused during compilation: '-Xthinlto-distributor=distarg2,distarg3'
+// NONE: ld.lld
+// NOMORE-NOT: distributor
+// NOMORE-NOT: remote-compiler
+
+
+/// Check the expected arguments are forwarded by default with only
+/// --thinlto-distributor=.
+// RUN: %clang --target=x86_64-linux-gnu -fthinlto-distributor=dist.exe \
+// RUN: -fuse-ld=lld -Werror -### %s 2>&1 | \
+// RUN: FileCheck %s --check-prefixes=DEFAULT,NOMORE
+
+// DEFAULT: ld.lld
+// DEFAULT-SAME: "--thinlto-distributor=dist.exe"
+// DEFAULT-SAME: "--thinlto-remote-compiler={{.*}}clang
diff --git a/cross-project-tests/dtlto/ld-dtlto.c b/cross-project-tests/dtlto/ld-dtlto.c
index 3ee962346bd4a..7dffe2e015bcb 100644
--- a/cross-project-tests/dtlto/ld-dtlto.c
+++ b/cross-project-tests/dtlto/ld-dtlto.c
@@ -5,13 +5,11 @@
// RUN: rm -rf %t && mkdir %t && cd %t
-// RUN: %clang --target=x86_64-linux-gnu -c -flto=thin %s -o dtlto.o
-
-// RUN: ld.lld dtlto.o \
-// RUN: --thinlto-distributor=%python \
-// RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \
-// RUN: --thinlto-remote-compiler=%clang \
-// RUN: --thinlto-remote-compiler-arg=--save-temps
+// RUN: %clang --target=x86_64-linux-gnu %s -flto=thin -fuse-ld=lld \
+// RUN: -fthinlto-distributor=%python \
+// RUN: -Xthinlto-distributor=%llvm_src_root/utils/dtlto/local.py \
+// RUN: -Wl,--thinlto-remote-compiler-arg=--save-temps \
+// RUN: -nostdlib -Werror
/// Check that the required output files have been created.
// RUN: ls | sort | FileCheck %s
@@ -22,18 +20,15 @@
/// Linked ELF.
// CHECK: {{^}}a.out{{$}}
-/// Produced by the bitcode compilation.
-// CHECK-NEXT: {{^}}dtlto.o{{$}}
-
/// --save-temps output for the backend compilation.
-// CHECK-NEXT: {{^}}dtlto.s{{$}}
-// CHECK-NEXT: {{^}}dtlto.s.0.preopt.bc{{$}}
-// CHECK-NEXT: {{^}}dtlto.s.1.promote.bc{{$}}
-// CHECK-NEXT: {{^}}dtlto.s.2.internalize.bc{{$}}
-// CHECK-NEXT: {{^}}dtlto.s.3.import.bc{{$}}
-// CHECK-NEXT: {{^}}dtlto.s.4.opt.bc{{$}}
-// CHECK-NEXT: {{^}}dtlto.s.5.precodegen.bc{{$}}
-// CHECK-NEXT: {{^}}dtlto.s.resolution.txt{{$}}
+// CHECK-NEXT: {{^}}ld-dtlto-[[TMP:[a-zA-Z0-9_]+]].s{{$}}
+// CHECK-NEXT: {{^}}ld-dtlto-[[TMP]].s.0.preopt.bc{{$}}
+// CHECK-NEXT: {{^}}ld-dtlto-[[TMP]].s.1.promote.bc{{$}}
+// CHECK-NEXT: {{^}}ld-dtlto-[[TMP]].s.2.internalize.bc{{$}}
+// CHECK-NEXT: {{^}}ld-dtlto-[[TMP]].s.3.import.bc{{$}}
+// CHECK-NEXT: {{^}}ld-dtlto-[[TMP]].s.4.opt.bc{{$}}
+// CHECK-NEXT: {{^}}ld-dtlto-[[TMP]].s.5.precodegen.bc{{$}}
+// CHECK-NEXT: {{^}}ld-dtlto-[[TMP]].s.resolution.txt{{$}}
/// No files are expected after.
// CHECK-NOT: {{.}}
|
bd1976bris
requested review from
MaskRay,
ilovepi,
tru,
justincady and
teresajohnson
clang/lib/Driver/ToolChains/Gnu.cpp
Outdated
| @@ -455,6 +455,21 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA, | |||
| addLTOOptions(ToolChain, Args, CmdArgs, Output, Inputs, | |||
| D.getLTOMode() == LTOK_Thin); | |||
|
|
|||
| // Forward the DTLTO options to the linker. We add these unconditionally, | |||
| // rather than in addLTOOptions() as it is the linker that decides whether to | |||
There was a problem hiding this comment.
Why not be consistent with the other LTO options though, which are added in addLTOOptions? The same argument could be made for those (and maybe they should be sent unconditionally, but that seems like a separate discussion).
There was a problem hiding this comment.
Apologies - we’ve recently started setting LTO options unconditionally in the PS5 driver, and I carried that pattern over here without fully reconsidering it. As you pointed out, the issue is broader than just the DTLTO options and deserves separate handling.
It’s also unclear whether setting options unconditionally is the right long-term approach. An alternative could be to pass a flag to the linker, allowing it to emit a warning if LTO is being performed but the Clang driver wasn't explicitly invoked in LTO mode.
In any case, I’ve moved the DTLTO option handling into addLTOOptions as you suggested. Thanks for the guidance.
clang/test/Driver/DTLTO/dtlto.c
Outdated
| /// that there is an unused option warning issued for -Xthinlto-distributor= | ||
| /// options. We specify -flto here as these options should be unaffected by it. | ||
| // RUN: %clang -### @%t.rsp -flto=thin %s 2>&1 | \ | ||
| // RUN: FileCheck %s --check-prefixes=NONE,NOMORE --implicit-check-not=warning |
There was a problem hiding this comment.
Confused about the --implicit-check-not=warning since there are some warnings below. Although I guess this is checking that there are none other than those being explicitly checked below - but is that intended?
There was a problem hiding this comment.
Yes that was the intent. I have now used a common response file for all FileCheck invocations in this test and added a comment to explain the intention behind the use of implicit-check-not arguments.
There was a problem hiding this comment.
For clang driver, prefer -Werror to implicit-check-not=warning: .
A warning would make clang -### -Werror exit with code 1.
There was a problem hiding this comment.
@MaskRay done. Thanks.
clang/test/Driver/DTLTO/dtlto.c
Outdated
| // NONE: warning: argument unused during compilation: '-Xthinlto-distributor=distarg1' | ||
| // NONE: warning: argument unused during compilation: '-Xthinlto-distributor=distarg2,distarg3' | ||
| // NONE: ld.lld | ||
| // NOMORE-NOT: distributor |
There was a problem hiding this comment.
Maybe better to put these in --implicit-check-not= since this would not catch any before the above warnings.
There was a problem hiding this comment.
Done. I have also used the same set of implicit-check-not arguments in each of the FileCheck invocations (via a response file) to reduce the size of the test and improve readability.
There was a problem hiding this comment.
@teresajohnson I have now removed the use of response files and implemented (judiciously) @MaskRay's suggestion of using -Werror.
ilovepi
reviewed
Jul 9, 2025
…when additional test cases are added.
bd1976bris
commented
Jul 11, 2025
ilovepi
reviewed
Jul 14, 2025
teresajohnson
approved these changes
Jul 14, 2025
|
Clang :: Driver/DTLTO/dtlto.c test is failing in our toolchain builders. See full log here: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8709270904931005265/+/u/clang/tests/stdout |
|
@Prabhuk I might need some help here. Looking at the output for the |
|
I see this patch also failing its own tests in our ci If this isn't trivial to fix, I'd suggest revert and reland once you've addressed it. |
We use the multicall build, so that is in fact our clang and lld (multicall is basically like a busybox config). clang and lld exist in the build and install directories as symlinks to the llvm binary. It's well supported in the CMake, and has been used by us and others for several years now. The toolchain we're building and testing is just a normal linux/windows/mac toolchain. Its configured to compile Fuchsia, but we don't enable anything too special, just some standard cmake options. |
@ilovepi - thanks very much! I think that it is impossible in general for me to predict the name of the tool that will be returned by |
|
Hi there, this PR failed the test on our bot breaking the builds. If it takes some time to land the fix, maybe consider revert and reland it? Thanks! bot: https://lab.llvm.org/buildbot/#/builders/10/builds/9468 |
bd1976bris
added a commit
to bd1976bris/llvm-project
that referenced
this pull request
Jul 15, 2025
Not all builds name the compiler executable `clang`. For example, the Fuchsia build bots use `llvm` as the executable name, as they combine everything together in a busybot-style binary. Update the Clang driver test to simply check that a non-empty path is provided for the `--thinlto-remote-compiler` argument, rather than hardcoding the executable name. The cross-project test will verify that the path is valid later. Should fix llvm#147265.
bd1976bris
added a commit
that referenced
this pull request
Jul 15, 2025
Not all builds name the compiler executable `clang`. For example, the Fuchsia build bots use `llvm` as their single toolchain executable name, as they combine everything together in a busybot-style binary. Update the Clang driver test to simply check that a non-empty path is provided for the `--thinlto-remote-compiler` argument, rather than hardcoding the executable name. The cross-project test will verify that the path is valid later. Should fix #147265.
|
Thanks! I appreciated the quick fix! |
|
@tru - would you be happy for me to backport this to the LLVM 21 release branch to complete the DTLTO feature? |
Yes! |
|
/pull-request #149978 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch introduces support for Integrated Distributed ThinLTO (DTLTO) in Clang.
DTLTO enables the distribution of ThinLTO backend compilations via external distribution systems, such as Incredibuild, during the traditional link step: https://llvm.org/docs/DTLTO.html.
Testing:
littest coverage has been added to Clang's Driver tests.For the design discussion of the DTLTO feature, see: #126654
Note that I have removed the forwarding of -mllvm options to the backend compilations which was discussed in the design review from this patch. LTO configuration for DTLTO will be addressed in a follow-up patch. In the meantime -mllvm options can be forwarded manually if required.