Skip to content

Conversation

@jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Jul 28, 2025

Summary:
Previously, querying for the offload architecture tool would invoke the
user's PATH, which is bad when potentially using the driver from a
direct path. This patch change this to only consider the
offload-arch that's supposed to live next to the driver executable.
Now we will no longer pick up a potentially conflicting version of this
tool and it should always be found (Since it's a clang tool that's
installazed alongside the driver)

Summary:
Previously, querying for the offload architecture tool would invoke the
user's PATH, which is bad when potentially using the driver from a
direct path. This patch change this to *only* consider the
`offload-arch` that's supposed to live next to the driver executable.
Now we will no longer pick up a potentially conflicting version of this
tool and it should always be found (Since it's a clang tool that's
installazed alongside the driver)
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jul 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Joseph Huber (jhuber6)

Changes

Summary:
Previously, querying for the offload architecture tool would invoke the
user's PATH, which is bad when potentially using the driver from a
direct path. This patch change this to only consider the
offload-arch that's supposed to live next to the driver executable.
Now we will no longer pick up a potentially conflicting version of this
tool and it should always be found (Since it's a clang tool that's
installazed alongside the driver)


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

1 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+1-1)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 853f694850ba8..b90bd1d9ea17b 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -910,7 +910,7 @@ getSystemOffloadArchs(Compilation &C, Action::OffloadKind Kind) {
 
   SmallVector<std::string> GPUArchs;
   if (llvm::ErrorOr<std::string> Executable =
-          llvm::sys::findProgramByName(Program)) {
+          llvm::sys::findProgramByName(Program, {C.getDriver().Dir})) {
     llvm::SmallVector<StringRef> Args{*Executable};
     if (Kind == Action::OFK_HIP)
       Args.push_back("--only=amdgpu");

Copy link
Contributor

@shiltian shiltian left a comment

Choose a reason for hiding this comment

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

Is clang tools always installed?

@jhuber6
Copy link
Contributor Author

jhuber6 commented Jul 28, 2025

Is clang tools always installed?

Yes, but if they somehow weren't this would just become an error for native detection, nothing catastrophic.

@jhuber6 jhuber6 merged commit 4f58c82 into llvm:main Jul 28, 2025
12 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.

3 participants