[HIP] Replace use of llvm-mc with clang
#112041
Merged
Conversation
This file contains 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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Joseph Huber (jhuber6) ChangesSummary: My preferred solution would be to use the new driver, but I still Fixes: #112031 Full diff: https://github.com/llvm/llvm-project/pull/112041.diff 9 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/HIPUtility.cpp b/clang/lib/Driver/ToolChains/HIPUtility.cpp
index b3adfe65402ff3..ffeea269a068ff 100644
--- a/clang/lib/Driver/ToolChains/HIPUtility.cpp
+++ b/clang/lib/Driver/ToolChains/HIPUtility.cpp
@@ -346,14 +346,14 @@ void HIP::constructGenerateObjFileFromHIPFatBinary(
// Create Temp Object File Generator,
// Offload Bundled file and Bundled Object file.
// Keep them if save-temps is enabled.
- const char *McinFile;
+ const char *ObjinFile;
const char *BundleFile;
if (C.getDriver().isSaveTempsEnabled()) {
- McinFile = C.getArgs().MakeArgString(Name + ".mcin");
+ ObjinFile = C.getArgs().MakeArgString(Name + ".mcin");
BundleFile = C.getArgs().MakeArgString(Name + ".hipfb");
} else {
auto TmpNameMcin = C.getDriver().GetTemporaryPath(Name, "mcin");
- McinFile = C.addTempFile(C.getArgs().MakeArgString(TmpNameMcin));
+ ObjinFile = C.addTempFile(C.getArgs().MakeArgString(TmpNameMcin));
auto TmpNameFb = C.getDriver().GetTemporaryPath(Name, "hipfb");
BundleFile = C.addTempFile(C.getArgs().MakeArgString(TmpNameFb));
}
@@ -454,7 +454,7 @@ void HIP::constructGenerateObjFileFromHIPFatBinary(
// Open script file and write the contents.
std::error_code EC;
- llvm::raw_fd_ostream Objf(McinFile, EC, llvm::sys::fs::OF_None);
+ llvm::raw_fd_ostream Objf(ObjinFile, EC, llvm::sys::fs::OF_None);
if (EC) {
C.getDriver().Diag(clang::diag::err_unable_to_make_temp) << EC.message();
@@ -463,10 +463,10 @@ void HIP::constructGenerateObjFileFromHIPFatBinary(
Objf << ObjBuffer;
- ArgStringList McArgs{"-triple", Args.MakeArgString(HostTriple.normalize()),
- "-o", Output.getFilename(),
- McinFile, "--filetype=obj"};
- const char *Mc = Args.MakeArgString(TC.GetProgramPath("llvm-mc"));
+ ArgStringList McArgs{"-target", Args.MakeArgString(HostTriple.normalize()),
+ "-o", Output.getFilename(), "-x", "assembler",
+ ObjinFile, "-c"};
+ const char *Mc = Args.MakeArgString(TC.GetProgramPath("clang"));
C.addCommand(std::make_unique<Command>(JA, T, ResponseFileSupport::None(), Mc,
McArgs, Inputs, Output));
}
diff --git a/clang/test/Driver/hip-link-save-temps.hip b/clang/test/Driver/hip-link-save-temps.hip
index 5656614626b9cd..e321970274bb4b 100644
--- a/clang/test/Driver/hip-link-save-temps.hip
+++ b/clang/test/Driver/hip-link-save-temps.hip
@@ -39,10 +39,10 @@
// CHECK-NOT: {{".*/opt"}}
// CHECK-NOT: {{".*/llc"}}
// CHECK: "{{.*lld.*}}" {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
-// CHECK-SAME: "-o" "a.out-hip-amdgcn-amd-amdhsa-gfx900" "obj1-hip-amdgcn-amd-amdhsa-gfx900.o" "obj2-hip-amdgcn-amd-amdhsa-gfx900.o"
+// CHECK-SAME: "-o" "[[HIPFB1:.+]]" "obj1-hip-amdgcn-amd-amdhsa-gfx900.o" "obj2-hip-amdgcn-amd-amdhsa-gfx900.o"
// CHECK: "{{.*lld.*}}" {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
-// CHECK-SAME: "-o" "a.out-hip-amdgcn-amd-amdhsa-gfx906" "obj1-hip-amdgcn-amd-amdhsa-gfx906.o" "obj2-hip-amdgcn-amd-amdhsa-gfx906.o"
-// CHECK: {{".*llvm-mc.*"}} "-o" "[[OBJBUNDLE:.*.o]]" "{{.*}}.mcin" "--filetype=obj"
+// CHECK-SAME: "-o" "[[HIPFB2:.+]]" "obj1-hip-amdgcn-amd-amdhsa-gfx906.o" "obj2-hip-amdgcn-amd-amdhsa-gfx906.o"
+// CHECK: "{{.*clang.*}}" "-target" "x86_64-unknown-linux-gnu" "-o" "[[OBJBUNDLE:.+.o]]" "-x" "assembler" "{{.*}}.mcin" "-c"
// OUT: "{{.*ld.*}}" {{.*}} "-o" "executable" {{.*}} "[[OBJBUNDLE]]"
// NOUT: "{{.*ld.*}}" {{.*}} "-o" "a.out" {{.*}} "[[OBJBUNDLE]]"
// SLO: "{{.*llvm-ar.*}}" "rcsD" "libTest.a" {{.*}} "[[OBJBUNDLE]]"
diff --git a/clang/test/Driver/hip-partial-link.hip b/clang/test/Driver/hip-partial-link.hip
index c8451ec81ed37e..8c244cd371f623 100644
--- a/clang/test/Driver/hip-partial-link.hip
+++ b/clang/test/Driver/hip-partial-link.hip
@@ -35,7 +35,7 @@
// LD-R: "{{.*}}/clang-offload-bundler" {{.*}}-unbundle
// LD-R: "{{.*}}/lld" -flavor gnu -m elf64_amdgpu
// LD-R: "{{.*}}/clang-offload-bundler"
-// LD-R: "{{.*}}/llvm-mc" -triple x86_64-unknown-linux-gnu
+// LD-R: "{{.*}}/clang" -target x86_64-unknown-linux-gnu
// LD-R: "{{.*}}/ld.lld" {{.*}} -r
// RUN: llvm-nm %t.lib.o | FileCheck -check-prefix=OBJ %s
@@ -65,7 +65,7 @@
// STATIC: "{{.*}}/clang-offload-bundler" {{.*}}-unbundle
// STATIC: "{{.*}}/lld" -flavor gnu -m elf64_amdgpu
// STATIC: "{{.*}}/clang-offload-bundler"
-// STATIC: "{{.*}}/llvm-mc" -triple x86_64-unknown-linux-gnu
+// STATIC: "{{.*}}/clang" -target x86_64-unknown-linux-gnu
// STATIC: "{{.*}}/llvm-ar"
// RUN: %clang -v --target=x86_64-unknown-linux-gnu --no-offload-new-driver \
diff --git a/clang/test/Driver/hip-save-temps.hip b/clang/test/Driver/hip-save-temps.hip
index 6dedfdafb11a13..142c3f1611a360 100644
--- a/clang/test/Driver/hip-save-temps.hip
+++ b/clang/test/Driver/hip-save-temps.hip
@@ -58,7 +58,7 @@
// RDCC: "{{.*clang.*}}" "-cc1as" {{.*}} "-o" "hip-save-temps-host-x86_64-unknown-linux-gnu.o"
// RDCC: "{{.*clang-offload-bundler.*}}" {{.*}} "-output=hip-save-temps.o"
// RDCL: "{{.*clang-offload-bundler.*}}" {{.*}} "-output=hip-save-temps-hip-amdgcn-amd-amdhsa.hipfb"
-// RDCL: {{.*}}llvm-mc{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa.o" "hip-save-temps-hip-amdgcn-amd-amdhsa.mcin" "--filetype=obj"
+// RDCL: "{{.*clang.*}}" "-target" "x86_64-unknown-linux-gnu" "-o" "{{.*}}.o" "-x" "assembler" "{{.*}}.mcin" "-c"
// -fno-gpu-rdc host object path
// NORDC: "{{.*clang.*}}" "-cc1" {{.*}} "-E" {{.*}} "-o" "hip-save-temps-host-x86_64-unknown-linux-gnu.hipi"
diff --git a/clang/test/Driver/hip-toolchain-rdc-separate.hip b/clang/test/Driver/hip-toolchain-rdc-separate.hip
index 92f493912adb4d..0ce5ea5174e1b1 100644
--- a/clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ b/clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -145,8 +145,8 @@
// LINK-BUNDLE-SAME: "-input={{.*}}" "-input=[[IMG_DEV1]]" "-input=[[IMG_DEV2]]" "-output=[[BUNDLE:.*]]"
// LINK-NOBUNDLE-NOT: {{".*clang-offload-bundler"}} "-type=o"
-// LINK-EMBED: {{".*llvm-mc.*"}} "-o" "[[OBJBUNDLE:.*o]]" "{{.*}}.mcin" "--filetype=obj"
-// LINK-NOEMBED-NOT: {{".*llvm-mc.*"}} "-o"
+// LINK-EMBED: {{".*clang.*"}} "-o" "[[OBJBUNDLE:.*o]]" "{{.*}}.mcin"
+// LINK-NOEMBED-NOT: {{".*clang.*"}} "-o"
// LINK-EMBED: [[LD:".*ld.*"]] {{.*}} "-o" "a.out" {{.*}} "[[A_OBJ_HOST]]"
// LINK-EMBED-SAME: "[[B_OBJ_HOST]]" "[[OBJBUNDLE]]"
diff --git a/clang/test/Driver/hip-toolchain-rdc-static-lib.hip b/clang/test/Driver/hip-toolchain-rdc-static-lib.hip
index 780426907e90e7..5276faf31bdc2b 100644
--- a/clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ b/clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -86,6 +86,6 @@
// CHECK-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
// CHECK-SAME: "-input=[[IMG_DEV1]]" "-input=[[IMG_DEV2]]" "-output=[[BUNDLE:.*hipfb]]"
-// CHECK: [[MC:".*llvm-mc.*"]] "-o" [[OBJBUNDLE:".*o"]] "{{.*}}.mcin" "--filetype=obj"
+// CHECK: [[MC:".*clang.*"]] "-o" [[OBJBUNDLE:".*o"]] "{{.*}}.mcin"
// CHECK: [[AR:".*llvm-ar.*"]] "rcsD" "{{.*}}.out" [[A_OBJ_HOST]] [[B_OBJ_HOST]] [[OBJBUNDLE]]
diff --git a/clang/test/Driver/hip-toolchain-rdc.hip b/clang/test/Driver/hip-toolchain-rdc.hip
index ec79bf06afb92c..6d3f46f8a94674 100644
--- a/clang/test/Driver/hip-toolchain-rdc.hip
+++ b/clang/test/Driver/hip-toolchain-rdc.hip
@@ -155,7 +155,7 @@
// CHECK-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
// CHECK-SAME: "-input={{.*}}" "-input=[[IMG_DEV1]]" "-input=[[IMG_DEV2]]" "-output=[[BUNDLE]]"
-// CHECK: [[MC:".*llvm-mc.*"]] "-triple" [[HOST]] "-o" [[OBJBUNDLE:".*o"]] "{{.*}}.mcin" "--filetype=obj"
+// CHECK: [[MC:".*clang.*"]] "-target" [[HOST]] "-o" [[OBJBUNDLE:".*o"]] "{{.*}}.mcin"
// output the executable
// LNX: [[LD:".*ld.*"]] {{.*}}"-o" "a.out" {{.*}} [[A_OBJ_HOST]] [[B_OBJ_HOST]] [[OBJBUNDLE]]
diff --git a/clang/test/Driver/hip-unbundle-preproc.hipi b/clang/test/Driver/hip-unbundle-preproc.hipi
index da5c68ef1c158d..a56cafea0e8d4e 100644
--- a/clang/test/Driver/hip-unbundle-preproc.hipi
+++ b/clang/test/Driver/hip-unbundle-preproc.hipi
@@ -23,5 +23,5 @@
// RDC: {{".*clang.*"}} "-cc1" {{.*}}"-target-cpu" "gfx803" {{.*}}"-o" "[[DEV_BC:.*bc]]" {{.*}}"[[DEV_PP]]"
// RDC: {{".*lld.*"}} {{.*}}"-o" "[[DEV_ISA:.*]]" "[[DEV_BC]]"
// RDC: {{".*clang-offload-bundler.*"}} {{.*}}"-input={{.*}}" "-input=[[DEV_ISA]]" "-output=[[FATBIN:.*]]"
-// RDC: {{".*llvm-mc.*"}} "-o" "[[FATBIN_O:.*o]]"
+// RDC: {{".*clang.*"}} "-o" "[[FATBIN_O:.*o]]"
// RDC: {{".*ld.*"}} {{.*}}"[[HOST_O]]" "[[FATBIN_O]]"
diff --git a/clang/test/Driver/hipspv-toolchain-rdc.hip b/clang/test/Driver/hipspv-toolchain-rdc.hip
index 2e7528a9996d87..acdadacc490649 100644
--- a/clang/test/Driver/hipspv-toolchain-rdc.hip
+++ b/clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -53,8 +53,7 @@
// CHECK-SAME: "-targets={{.*}},hip-spirv64----generic"
// CHECK-SAME: "-input=/dev/null" "-input=[[AB_SPIRV]]"
// CHECK-SAME: "-output=[[AB_FATBIN:.*hipfb]]"
-// CHECK: {{".*llvm-mc.*"}} "-o" [[OBJBUNDLE:".*o"]] "{{.*}}.mcin"
-// CHECK-SAME: "--filetype=obj"
+// CHECK: {{".*clang.*"}} "-o" [[OBJBUNDLE:".*o"]] "{{.*}}.mcin"
// Output the executable
// CHECK: {{".*ld.*"}} {{.*}}"-o" "a.out" {{.*}} [[A_OBJ_HOST]] [[B_OBJ_HOST]]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Summary: We currently use `llvm-mc` which is intended for internal testing and not expected to be present in every installation. This patch changes that to just use clang instead to get the `.o` from the HIP registration code. My preferred solution would be to use the new driver, but I still haven't gotten the test suite to pass on this one weird OpenMP case. Fixes: llvm#112031
arsenm
reviewed
Oct 12, 2024
arsenm
approved these changes
Oct 12, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/2444 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/3148 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/4269 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/5057 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/4845 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/6522 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/4368 Here is the relevant piece of the build log for the reference |
|
@petrhosek Can someone explain why the clang executable in these tests is called |
We enable the |
Interesting, right now it just takes |
|
This seems to be failing all of the precommit CI pipelines in Clang: Can this be fixed or reverted? |
I'm confused, that test was modified in this patch and no longer contains the |
|
I fixed an apparent missing test dependency in 1ac6ef5 and this commit removed the llvm-mc dependency |
I probably should've remembered to include removing that in this patch. |
|
Thanks, things seem to be better now! |
I think some other hexagon code is also using llvm-mc |
It should be |
The variable used here is initialized via |
petrhosek
added a commit
to petrhosek/llvm-project
that referenced
this pull request
Oct 14, 2024
This addresses an issue introduced in llvm#112041.
petrhosek
added a commit
that referenced
this pull request
Oct 14, 2024
This addresses an issue introduced in #112041.
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
Summary: We currently use `llvm-mc` which is intended for internal testing and not expected to be present in every installation. This patch changes that to just use clang instead to get the `.o` from the HIP registration code. My preferred solution would be to use the new driver, but I still haven't gotten the test suite to pass on this one weird OpenMP case. Fixes: llvm#112031
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
This addresses an issue introduced in llvm#112041.
Xeonacid
added a commit
that referenced
this pull request
Jan 22, 2025
#112041 replaced `llvm-mc` with `clang`. The args are now feeding to clang.
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.
Summary:
We currently use
llvm-mcwhich is intended for internal testing andnot expected to be present in every installation. This patch changes
that to just use clang instead to get the
.ofrom the HIP registrationcode.
My preferred solution would be to use the new driver, but I still
haven't gotten the test suite to pass on this one weird OpenMP case.
Fixes: #112031