-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[HIP][SPIRV] Create an intermediate file for the llvm-link output #162096
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Before this patch we had: clang -cc1 source.c -o bitcode.bc llvm-link -o a.out bitcode.bc llvm-spirv -o a.out a.out Now we have: clang -cc1 source.c -o bitcode.bc llvm-link -o a-linked.bc bitcode.bc llvm-spirv -o a.out a-linked.bc Co-authored-by: Manuel Carrasco <[email protected]>
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Juan Manuel Martinez Caamaño (jmmartinez) ChangesBefore this patch we had: Now we have: Co-authored-by: Manuel Carrasco <[email protected]> Full diff: https://github.com/llvm/llvm-project/pull/162096.diff 6 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index 5f3fbea40f162..c0c8afec07264 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -168,9 +168,12 @@ void AMDGCN::Linker::constructLinkAndEmitSpirvCommand(
const InputInfo &Output, const llvm::opt::ArgList &Args) const {
assert(!Inputs.empty() && "Must have at least one input.");
- constructLlvmLinkCommand(C, JA, Inputs, Output, Args);
+ std::string LinkedBCFilePrefix(
+ Twine(llvm::sys::path::stem(Output.getFilename()), "-linked").str());
+ const char *LinkedBCFilePath = HIP::getTempFile(C, LinkedBCFilePrefix, "bc");
+ InputInfo LinkedBCFile(&JA, LinkedBCFilePath, Output.getBaseInput());
- // Linked BC is now in Output
+ constructLlvmLinkCommand(C, JA, Inputs, LinkedBCFile, Args);
// Emit SPIR-V binary.
llvm::opt::ArgStringList TrArgs{
@@ -180,7 +183,7 @@ void AMDGCN::Linker::constructLinkAndEmitSpirvCommand(
"--spirv-lower-const-expr",
"--spirv-preserve-auxdata",
"--spirv-debug-info-version=nonsemantic-shader-200"};
- SPIRV::constructTranslateCommand(C, *this, JA, Output, Output, TrArgs);
+ SPIRV::constructTranslateCommand(C, *this, JA, Output, LinkedBCFile, TrArgs);
}
// For amdgcn the inputs of the linker job are device bitcode and output is
diff --git a/clang/lib/Driver/ToolChains/HIPSPV.cpp b/clang/lib/Driver/ToolChains/HIPSPV.cpp
index 62bca0493a09a..bce7f46dea468 100644
--- a/clang/lib/Driver/ToolChains/HIPSPV.cpp
+++ b/clang/lib/Driver/ToolChains/HIPSPV.cpp
@@ -22,17 +22,6 @@ using namespace clang::driver::tools;
using namespace clang;
using namespace llvm::opt;
-// Convenience function for creating temporary file for both modes of
-// isSaveTempsEnabled().
-static const char *getTempFile(Compilation &C, StringRef Prefix,
- StringRef Extension) {
- if (C.getDriver().isSaveTempsEnabled()) {
- return C.getArgs().MakeArgString(Prefix + "." + Extension);
- }
- auto TmpFile = C.getDriver().GetTemporaryPath(Prefix, Extension);
- return C.addTempFile(C.getArgs().MakeArgString(TmpFile));
-}
-
// Locates HIP pass plugin.
static std::string findPassPlugin(const Driver &D,
const llvm::opt::ArgList &Args) {
@@ -65,7 +54,7 @@ void HIPSPV::Linker::constructLinkAndEmitSpirvCommand(
assert(!Inputs.empty() && "Must have at least one input.");
std::string Name = std::string(llvm::sys::path::stem(Output.getFilename()));
- const char *TempFile = getTempFile(C, Name + "-link", "bc");
+ const char *TempFile = HIP::getTempFile(C, Name + "-link", "bc");
// Link LLVM bitcode.
ArgStringList LinkArgs{};
@@ -93,7 +82,7 @@ void HIPSPV::Linker::constructLinkAndEmitSpirvCommand(
auto PassPluginPath = findPassPlugin(C.getDriver(), Args);
if (!PassPluginPath.empty()) {
const char *PassPathCStr = C.getArgs().MakeArgString(PassPluginPath);
- const char *OptOutput = getTempFile(C, Name + "-lower", "bc");
+ const char *OptOutput = HIP::getTempFile(C, Name + "-lower", "bc");
ArgStringList OptArgs{TempFile, "-load-pass-plugin",
PassPathCStr, "-passes=hip-post-link-passes",
"-o", OptOutput};
diff --git a/clang/lib/Driver/ToolChains/HIPUtility.cpp b/clang/lib/Driver/ToolChains/HIPUtility.cpp
index cb061ffede234..732403e69a075 100644
--- a/clang/lib/Driver/ToolChains/HIPUtility.cpp
+++ b/clang/lib/Driver/ToolChains/HIPUtility.cpp
@@ -472,3 +472,14 @@ void HIP::constructGenerateObjFileFromHIPFatBinary(
D.getClangProgramPath(), ClangArgs,
Inputs, Output, D.getPrependArg()));
}
+
+// Convenience function for creating temporary file for both modes of
+// isSaveTempsEnabled().
+const char *HIP::getTempFile(Compilation &C, StringRef Prefix,
+ StringRef Extension) {
+ if (C.getDriver().isSaveTempsEnabled()) {
+ return C.getArgs().MakeArgString(Prefix + "." + Extension);
+ }
+ auto TmpFile = C.getDriver().GetTemporaryPath(Prefix, Extension);
+ return C.addTempFile(C.getArgs().MakeArgString(TmpFile));
+}
diff --git a/clang/lib/Driver/ToolChains/HIPUtility.h b/clang/lib/Driver/ToolChains/HIPUtility.h
index 29e5a922024ab..55c155e5c35cf 100644
--- a/clang/lib/Driver/ToolChains/HIPUtility.h
+++ b/clang/lib/Driver/ToolChains/HIPUtility.h
@@ -16,6 +16,8 @@ namespace driver {
namespace tools {
namespace HIP {
+const char *getTempFile(Compilation &C, StringRef Prefix, StringRef Extension);
+
// Construct command for creating HIP fatbin.
void constructHIPFatbinCommand(Compilation &C, const JobAction &JA,
StringRef OutputFileName,
diff --git a/clang/test/Driver/hip-toolchain-no-rdc.hip b/clang/test/Driver/hip-toolchain-no-rdc.hip
index dc8f0a97ad371..a9e7de9aa0040 100644
--- a/clang/test/Driver/hip-toolchain-no-rdc.hip
+++ b/clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -207,7 +207,7 @@
//
// AMDGCNSPIRV: "-cc1" "-triple" "spirv64-amd-amdhsa" {{.*}}"-emit-llvm-bc" {{.*}}"-fembed-bitcode=marker" "-disable-llvm-passes" {{.*}} "-o" "[[AMDGCNSPV_BC:.*bc]]"
-// AMDGCNSPIRV: {{".*llvm-link.*"}} "-o" "[[AMDGCNSPV_TMP:.*out]]" "[[AMDGCNSPV_BC]]"
+// AMDGCNSPIRV: {{".*llvm-link.*"}} "-o" "[[AMDGCNSPV_TMP:.*bc]]" "[[AMDGCNSPV_BC]]"
// AMDGCNSPIRV: {{".*llvm-spirv.*"}} "--spirv-max-version=1.6" "--spirv-ext=+all" {{.*}} "[[AMDGCNSPV_TMP]]" {{.*}}"-o" "[[AMDGCNSPV_CO:.*out]]"
// AMDGCNSPIRV: "-cc1" "-triple" "amdgcn-amd-amdhsa" {{.*}}"-emit-obj" {{.*}}"-target-cpu" "gfx900"{{.*}} "-o" "[[GFX900_OBJ:.*o]]"
// AMDGCNSPIRV: {{".*lld.*"}} {{.*}}"-plugin-opt=mcpu=gfx900" {{.*}} "-o" "[[GFX900_CO:.*out]]" {{.*}}"[[GFX900_OBJ]]"
diff --git a/clang/test/Driver/spirv-amd-toolchain.c b/clang/test/Driver/spirv-amd-toolchain.c
index 14ba8f4632477..8f1f0f33e53f9 100644
--- a/clang/test/Driver/spirv-amd-toolchain.c
+++ b/clang/test/Driver/spirv-amd-toolchain.c
@@ -15,5 +15,5 @@
// RUN: %clang -### --target=spirv64-amd-amdhsa %s -nogpulib -nogpuinc 2>&1 \
// RUN: | FileCheck %s --check-prefix=INVOCATION
// INVOCATION: "-cc1" "-triple" "spirv64-amd-amdhsa" {{.*}}"-disable-llvm-optzns" {{.*}} "-o" "[[OUTPUT:.+]]" "-x" "c"
-// INVOCATION: "{{.*}}llvm-link" "-o" "a.out" "[[OUTPUT]]"
-// INVOCATION: "{{.*}}llvm-spirv" "--spirv-max-version=1.6" "--spirv-ext=+all" "--spirv-allow-unknown-intrinsics" "--spirv-lower-const-expr" "--spirv-preserve-auxdata" "--spirv-debug-info-version=nonsemantic-shader-200" "a.out" "-o" "a.out"
+// INVOCATION: "{{.*}}llvm-link" "-o" "[[LINKED_OUTPUT:.+]]" "[[OUTPUT]]"
+// INVOCATION: "{{.*}}llvm-spirv" "--spirv-max-version=1.6" "--spirv-ext=+all" "--spirv-allow-unknown-intrinsics" "--spirv-lower-const-expr" "--spirv-preserve-auxdata" "--spirv-debug-info-version=nonsemantic-shader-200" "[[LINKED_OUTPUT]]" "-o" "a.out"
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, can you wait a day or two before merging to see if anyone else has feedback?
Thanks
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/23175 Here is the relevant piece of the build log for the reference
|
…62096) Before this patch we had: > clang -cc1 source.c -o bitcode.bc > llvm-link -o a.out bitcode.bc > llvm-spirv -o a.out a.out Now we have: > clang -cc1 source.c -o bitcode.bc > llvm-link -o a-linked.bc bitcode.bc > llvm-spirv -o a.out a-linked.bc Co-authored-by: Manuel Carrasco <[email protected]>
Before this patch we had:
Now we have:
Co-authored-by: Manuel Carrasco [email protected]