Skip to content

Conversation

@yxsamliu
Copy link
Collaborator

@yxsamliu yxsamliu commented Apr 10, 2025

for specifying number of threads for clang-linker-wrapper.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Apr 10, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 3 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • clang/include/clang/Driver/Options.td: Language not supported
  • clang/test/Driver/hip-options.hip: Language not supported
Comments suppressed due to low confidence (1)

clang/lib/Driver/ToolChains/Clang.cpp:9373

  • Consider whether the condition 'if (NumThreads > 1)' is intended. If a computed thread count of 1 is valid, you might want to explicitly pass '--wrapper-jobs=1' to ensure consistent behavior.
if (NumThreads > 1)

@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2025

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Yaxun (Sam) Liu (yxsamliu)

Changes

for specifying number of threads for clang-linker-wrapper. By default it uses half of hardware threads.


Full diff: https://github.com/llvm/llvm-project/pull/135229.diff

3 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+4)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+15)
  • (modified) clang/test/Driver/hip-options.hip (+9-2)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2ca5f99e4ca63..3b168f969dd28 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1233,6 +1233,10 @@ def offload_compression_level_EQ : Joined<["--"], "offload-compression-level=">,
   Flags<[HelpHidden]>,
   HelpText<"Compression level for offload device binaries (HIP only)">;
 
+def offload_jobs_EQ : Joined<["--"], "offload-jobs=">,
+  HelpText<"Set the number of threads for the clang-linker-wrapper. Defaults to"
+           " half of the available hardware threads if not specified.">;
+
 defm offload_via_llvm : BoolFOption<"offload-via-llvm",
   LangOpts<"OffloadViaLLVM">, DefaultFalse,
   PosFlag<SetTrue, [], [ClangOption, CC1Option], "Use">,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 13b7b94424999..1109386ac5f4c 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -57,6 +57,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/YAMLParser.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/TargetParser/ARMTargetParserCommon.h"
@@ -1031,6 +1032,7 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
     if (JA.isOffloading(Action::OFK_HIP)) {
       Args.ClaimAllArgs(options::OPT_offload_compress);
       Args.ClaimAllArgs(options::OPT_no_offload_compress);
+      Args.ClaimAllArgs(options::OPT_offload_jobs_EQ);
     }
 
     bool HasTarget = false;
@@ -9360,6 +9362,19 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
       CmdArgs.push_back(LinkArg);
 
   addOffloadCompressArgs(Args, CmdArgs);
+
+  // Default to half of hardware threads if users do not specify it.
+  if (Arg *A = Args.getLastArg(options::OPT_offload_jobs_EQ))
+    CmdArgs.push_back(
+        Args.MakeArgString(Twine("--wrapper-jobs=") + A->getValue()));
+  else {
+    unsigned NumThreads =
+        llvm::hardware_concurrency().compute_thread_count() / 2;
+    if (NumThreads > 1)
+      CmdArgs.push_back(
+          Args.MakeArgString(Twine("--wrapper-jobs=") + Twine(NumThreads)));
+  }
+
   const char *Exec =
       Args.MakeArgString(getToolChain().GetProgramPath("clang-linker-wrapper"));
 
diff --git a/clang/test/Driver/hip-options.hip b/clang/test/Driver/hip-options.hip
index 29d23c1b6c8d9..c9f141cb13d7c 100644
--- a/clang/test/Driver/hip-options.hip
+++ b/clang/test/Driver/hip-options.hip
@@ -243,6 +243,13 @@
 // NO-WARN-ATOMIC-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-Werror=atomic-alignment"
 // NO-WARN-ATOMIC-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-Wno-error=atomic-alignment"
 
-// Check --offload-compress does not cause warning.
+// Check --offload-compress --offload-jobs=N does not cause warning.
 // RUN: %clang -### -Werror --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
-// RUN:   --offload-arch=gfx1100 --offload-compress --offload-host-only -M %s
+// RUN:   --offload-arch=gfx1100 --offload-compress --offload-host-only -M %s \
+// RUN:   --offload-jobs=4
+
+// Check --offload-jobs=N passed to linker wrapper.
+// RUN: %clang -### -Werror --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx1100 --offload-new-driver --offload-jobs=4 %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=JOBS %s
+// JOBS: clang-linker-wrapper{{.*}} "--wrapper-jobs=4"

@yxsamliu yxsamliu requested a review from Artem-B April 10, 2025 18:25
Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG with one nit.

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, one nit.

for specifying number of threads for clang-linker-wrapper.
@yxsamliu yxsamliu merged commit 9332f1e into llvm:main Apr 14, 2025
7 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants