[clang] [OpenMP] Fix passing target id features to AMDGPU offloading (PR #94765)
https://github.com/jhuber6 closed https://github.com/llvm/llvm-project/pull/94765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Fix passing target id features to AMDGPU offloading (PR #94765)
https://github.com/yxsamliu approved this pull request. https://github.com/llvm/llvm-project/pull/94765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Fix passing target id features to AMDGPU offloading (PR #94765)
llvmbot wrote:
@llvm/pr-subscribers-backend-amdgpu
Author: Joseph Huber (jhuber6)
Changes
Summary:
AMDGPU supports a `target-id` feature which is used to qualify targets
with different incompatible features. These are both rules and target
features. Currently, we pass `-target-cpu` twice when offloading to
OpenMP, and do not pass the target-id features at all. The effect was
that passing something like `--offload-arch=gfx90a:xnack+` would show up
as `-target-cpu=gfx90a:xnack+ -target-cpu=gfx90a`. Thus ignoring the
xnack completely and passing it twice. This patch fixes that to pass it
once and then separate it like how HIP does.
---
Full diff: https://github.com/llvm/llvm-project/pull/94765.diff
3 Files Affected:
- (modified) clang/lib/Driver/ToolChains/AMDGPU.cpp (+5-1)
- (modified) clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp (-5)
- (modified) clang/test/Driver/amdgpu-openmp-toolchain.c (+2-1)
``diff
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 11a98a0ec314d..20f879e2f75cb 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -645,7 +645,11 @@ void amdgpu::getAMDGPUTargetFeatures(const Driver ,
std::vector ) {
// Add target ID features to -target-feature options. No diagnostics should
// be emitted here since invalid target ID is diagnosed at other places.
- StringRef TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
+ StringRef TargetID;
+ if (Args.hasArg(options::OPT_mcpu_EQ))
+TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
+ else if (Args.hasArg(options::OPT_march_EQ))
+TargetID = Args.getLastArgValue(options::OPT_march_EQ);
if (!TargetID.empty()) {
llvm::StringMap FeatureMap;
auto OptionalGpuArch = parseTargetID(Triple, TargetID, );
diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index cca18431ff773..d17ecb15c8208 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -44,14 +44,9 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
Action::OffloadKind DeviceOffloadingKind) const {
HostTC.addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadingKind);
- StringRef GPUArch = DriverArgs.getLastArgValue(options::OPT_march_EQ);
- assert(!GPUArch.empty() && "Must have an explicit GPU arch.");
-
assert(DeviceOffloadingKind == Action::OFK_OpenMP &&
"Only OpenMP offloading kinds are supported.");
- CC1Args.push_back("-target-cpu");
- CC1Args.push_back(DriverArgs.MakeArgStringRef(GPUArch));
CC1Args.push_back("-fcuda-is-device");
if (DriverArgs.hasArg(options::OPT_nogpulib))
diff --git a/clang/test/Driver/amdgpu-openmp-toolchain.c
b/clang/test/Driver/amdgpu-openmp-toolchain.c
index ef58c2c4e3f3a..49af04acc4639 100644
--- a/clang/test/Driver/amdgpu-openmp-toolchain.c
+++ b/clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -7,7 +7,7 @@
// verify the tools invocations
// CHECK: "-cc1" "-triple"
"x86_64-unknown-linux-gnu"{{.*}}"-emit-llvm-bc"{{.*}}"-x" "c"
-// CHECK: "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple"
"x86_64-unknown-linux-gnu"{{.*}}"-target-cpu"
"gfx906"{{.*}}"-fcuda-is-device"{{.*}}
+// CHECK: "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple"
"x86_64-unknown-linux-gnu"{{.*}}"-fcuda-is-device"{{.*}}"-target-cpu" "gfx906"
// CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-obj"
// CHECK: clang-linker-wrapper{{.*}} "-o" "a.out"
@@ -63,6 +63,7 @@
// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp
--offload-arch=gfx90a:sramecc-:xnack+ \
// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-TARGET-ID
+// CHECK-TARGET-ID: "-cc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-target-cpu"
"gfx90a" "-target-feature" "-sramecc" "-target-feature" "+xnack"
// CHECK-TARGET-ID:
clang-offload-packager{{.*}}arch=gfx90a:sramecc-:xnack+,kind=openmp,feature=-sramecc,feature=+xnack
// RUN: not %clang -### -target x86_64-pc-linux-gnu -fopenmp
--offload-arch=gfx90a,gfx90a:xnack+ \
``
https://github.com/llvm/llvm-project/pull/94765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Fix passing target id features to AMDGPU offloading (PR #94765)
https://github.com/jhuber6 created
https://github.com/llvm/llvm-project/pull/94765
Summary:
AMDGPU supports a `target-id` feature which is used to qualify targets
with different incompatible features. These are both rules and target
features. Currently, we pass `-target-cpu` twice when offloading to
OpenMP, and do not pass the target-id features at all. The effect was
that passing something like `--offload-arch=gfx90a:xnack+` would show up
as `-target-cpu=gfx90a:xnack+ -target-cpu=gfx90a`. Thus ignoring the
xnack completely and passing it twice. This patch fixes that to pass it
once and then separate it like how HIP does.
>From 9c91b92e62c60720fbd142660fd723dd1838400a Mon Sep 17 00:00:00 2001
From: Joseph Huber
Date: Fri, 7 Jun 2024 10:59:26 -0500
Subject: [PATCH] [OpenMP] Fix passing target id features to AMDGPU offloading
Summary:
AMDGPU supports a `target-id` feature which is used to qualify targets
with different incompatible features. These are both rules and target
features. Currently, we pass `-target-cpu` twice when offloading to
OpenMP, and do not pass the target-id features at all. The effect was
that passing something like `--offload-arch=gfx90a:xnack+` would show up
as `-target-cpu=gfx90a:xnack+ -target-cpu=gfx90a`. Thus ignoring the
xnack completely and passing it twice. This patch fixes that to pass it
once and then separate it like how HIP does.
---
clang/lib/Driver/ToolChains/AMDGPU.cpp | 6 +-
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp | 5 -
clang/test/Driver/amdgpu-openmp-toolchain.c | 3 ++-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 11a98a0ec314d..20f879e2f75cb 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -645,7 +645,11 @@ void amdgpu::getAMDGPUTargetFeatures(const Driver ,
std::vector ) {
// Add target ID features to -target-feature options. No diagnostics should
// be emitted here since invalid target ID is diagnosed at other places.
- StringRef TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
+ StringRef TargetID;
+ if (Args.hasArg(options::OPT_mcpu_EQ))
+TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
+ else if (Args.hasArg(options::OPT_march_EQ))
+TargetID = Args.getLastArgValue(options::OPT_march_EQ);
if (!TargetID.empty()) {
llvm::StringMap FeatureMap;
auto OptionalGpuArch = parseTargetID(Triple, TargetID, );
diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index cca18431ff773..d17ecb15c8208 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -44,14 +44,9 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
Action::OffloadKind DeviceOffloadingKind) const {
HostTC.addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadingKind);
- StringRef GPUArch = DriverArgs.getLastArgValue(options::OPT_march_EQ);
- assert(!GPUArch.empty() && "Must have an explicit GPU arch.");
-
assert(DeviceOffloadingKind == Action::OFK_OpenMP &&
"Only OpenMP offloading kinds are supported.");
- CC1Args.push_back("-target-cpu");
- CC1Args.push_back(DriverArgs.MakeArgStringRef(GPUArch));
CC1Args.push_back("-fcuda-is-device");
if (DriverArgs.hasArg(options::OPT_nogpulib))
diff --git a/clang/test/Driver/amdgpu-openmp-toolchain.c
b/clang/test/Driver/amdgpu-openmp-toolchain.c
index ef58c2c4e3f3a..49af04acc4639 100644
--- a/clang/test/Driver/amdgpu-openmp-toolchain.c
+++ b/clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -7,7 +7,7 @@
// verify the tools invocations
// CHECK: "-cc1" "-triple"
"x86_64-unknown-linux-gnu"{{.*}}"-emit-llvm-bc"{{.*}}"-x" "c"
-// CHECK: "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple"
"x86_64-unknown-linux-gnu"{{.*}}"-target-cpu"
"gfx906"{{.*}}"-fcuda-is-device"{{.*}}
+// CHECK: "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple"
"x86_64-unknown-linux-gnu"{{.*}}"-fcuda-is-device"{{.*}}"-target-cpu" "gfx906"
// CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-obj"
// CHECK: clang-linker-wrapper{{.*}} "-o" "a.out"
@@ -63,6 +63,7 @@
// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp
--offload-arch=gfx90a:sramecc-:xnack+ \
// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-TARGET-ID
+// CHECK-TARGET-ID: "-cc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-target-cpu"
"gfx90a" "-target-feature" "-sramecc" "-target-feature" "+xnack"
// CHECK-TARGET-ID:
clang-offload-packager{{.*}}arch=gfx90a:sramecc-:xnack+,kind=openmp,feature=-sramecc,feature=+xnack
// RUN: not %clang -### -target x86_64-pc-linux-gnu -fopenmp
--offload-arch=gfx90a,gfx90a:xnack+ \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
