[SPIR-V] Prefer llvm-spirv-<LLVM_VERSION_MAJOR> tool #77897
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-backend-spir-v Author: Henry Linjamäki (linehill) ChangesPrefer using An issue with the using Full diff: https://github.com/llvm/llvm-project/pull/77897.diff 4 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/SPIRV.cpp b/clang/lib/Driver/ToolChains/SPIRV.cpp
index 27de69550853cf..ce900600cbee51 100644
--- a/clang/lib/Driver/ToolChains/SPIRV.cpp
+++ b/clang/lib/Driver/ToolChains/SPIRV.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "SPIRV.h"
#include "CommonArgs.h"
+#include "clang/Basic/Version.h"
#include "clang/Driver/Compilation.h"
#include "clang/Driver/Driver.h"
#include "clang/Driver/InputInfo.h"
@@ -32,8 +33,15 @@ void SPIRV::constructTranslateCommand(Compilation &C, const Tool &T,
CmdArgs.append({"-o", Output.getFilename()});
- const char *Exec =
- C.getArgs().MakeArgString(T.getToolChain().GetProgramPath("llvm-spirv"));
+ // Try to find "llvm-spirv-<LLVM_VERSION_MAJOR>". Otherwise, fall back to
+ // plain "llvm-spirv".
+ using namespace std::string_literals;
+ auto VersionedTool = "llvm-spirv-"s + std::to_string(LLVM_VERSION_MAJOR);
+ std::string ExeCand = T.getToolChain().GetProgramPath(VersionedTool.c_str());
+ if (!llvm::sys::fs::can_execute(ExeCand))
+ ExeCand = T.getToolChain().GetProgramPath("llvm-spirv");
+
+ const char *Exec = C.getArgs().MakeArgString(ExeCand);
C.addCommand(std::make_unique<Command>(JA, T, ResponseFileSupport::None(),
Exec, CmdArgs, Input, Output));
}
diff --git a/clang/test/Driver/hipspv-toolchain.hip b/clang/test/Driver/hipspv-toolchain.hip
index bfae41049ba4dd..5f6dcc069afe8b 100644
--- a/clang/test/Driver/hipspv-toolchain.hip
+++ b/clang/test/Driver/hipspv-toolchain.hip
@@ -34,3 +34,16 @@
// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
+
+//-----------------------------------------------------------------------------
+// Check llvm-spirv-<LLVM_VERSION_MAJOR> is used if it is found in PATH.
+// RUN: mkdir -p %t/versioned
+// RUN: touch %t/versioned/llvm-spirv-%llvm-version-major \
+// RUN: && chmod +x %t/versioned/llvm-spirv-%llvm-version-major
+// RUN: env "PATH=%t/versioned" %clang -### -target x86_64-linux-gnu \
+// RUN: --offload=spirv64 --hip-path=%S/Inputs/hipspv -nohipwrapperinc \
+// RUN: %s 2>&1 \
+// RUN: | FileCheck -DVERSION=%llvm-version-major \
+// RUN: --check-prefix=VERSIONED %s
+
+// VERSIONED: {{.*}}llvm-spirv-[[VERSION]]
diff --git a/clang/test/Driver/spirv-toolchain.cl b/clang/test/Driver/spirv-toolchain.cl
index db3ee4d3fe02f8..de818177cb19f1 100644
--- a/clang/test/Driver/spirv-toolchain.cl
+++ b/clang/test/Driver/spirv-toolchain.cl
@@ -77,3 +77,13 @@
// XTOR: {{llvm-spirv.*"}}
// BACKEND-NOT: {{llvm-spirv.*"}}
+
+//-----------------------------------------------------------------------------
+// Check llvm-spirv-<LLVM_VERSION_MAJOR> is used if it is found in PATH.
+// RUN: mkdir -p %t/versioned
+// RUN: touch %t/versioned/llvm-spirv-%llvm-version-major \
+// RUN: && chmod +x %t/versioned/llvm-spirv-%llvm-version-major
+// RUN: env "PATH=%t/versioned" %clang -### --target=spirv64 -x cl -c %s 2>&1 \
+// RUN: | FileCheck -DVERSION=%llvm-version-major --check-prefix=VERSIONED %s
+
+// VERSIONED: {{.*}}llvm-spirv-[[VERSION]]
diff --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index ef75770a2c3c9a..b4e88096aaa0f3 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -41,6 +41,7 @@ config.llvm_external_lit = path(r"@LLVM_EXTERNAL_LIT@")
config.standalone_build = @CLANG_BUILT_STANDALONE@
config.ppc_linux_default_ieeelongdouble = @PPC_LINUX_DEFAULT_IEEELONGDOUBLE@
config.have_llvm_driver = @LLVM_TOOL_LLVM_DRIVER_BUILD@
+config.substitutions.append(("%llvm-version-major", "@LLVM_VERSION_MAJOR@"))
import lit.llvm
lit.llvm.initialize(lit_config, config)
|
|
@AnastasiaStulova, @MaskRay, gentle ping. |
|
Thanks for the review. Could you merge this PR on my behalf (I don't have write access)? |
Yes I am happy to do so, but would you mind rebasing this PR onto latest |
Prefer using `llvm-spirv-<LLVM_VERSION_MAJOR>` tool (i.e. `llvm-spirv-18`) over plain `llvm-spirv`. If the versioned tool is not found in PATH, fall back to use the plain `llvm-spirv`. An issue with the using `llvm-spirv` is that the one found in PATH might be compiled against older LLVM version which could lead to crashes or obscure bugs. For example, `llvm-spirv` distributed by Ubuntu links against different LLVM version depending on the Ubuntu release (LLVM-10 in 20.04LTS, LLVM-13 in 22.04LTS).
linehill
force-pushed
the
llvm-spirv-version
branch
from
fc1e2d0 to
28c8da4
Compare
|
Ping, @svenvh. The patch has been rebased, good for landing? |
Thanks for the ping, it seems github didn't send me a notification when you rebased. Just waiting for the final checks to complete, then I'll merge it. |
|
Thanks, @svenvh. |
Prefer using
llvm-spirv-<LLVM_VERSION_MAJOR>tool (i.e.llvm-spirv-18) over plainllvm-spirv. If the versioned tool is not found in PATH, fall back to use the plainllvm-spirv.An issue with the using
llvm-spirvis that the one found in PATH might be compiled against older LLVM version which could lead to crashes or obscure bugs. For example,llvm-spirvdistributed by Ubuntu links against different LLVM version depending on the Ubuntu release (LLVM-10 in 20.04LTS, LLVM-13 in 22.04LTS).