Skip to content
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

[OpenMP][AMDGPU] Do not attach -fcuda-is-device #99002

Merged

Conversation

DominikAdamski
Copy link
Contributor

-fcuda-is-device flag is not used for OpenMP offloading for AMD GPUs and it does not need to be added as clang cc1 option for OpenMP code.

This PR has the same functionality as #96909 but it doesn't introduce regression for virtual function support.

-fcuda-is-device flag is not used for OpenMP offloading for AMD GPUs
and it does not need to be added as clang cc1 option for OpenMP code.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AMDGPU clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:codegen clang:openmp OpenMP related changes to Clang labels Jul 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 16, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang-codegen

Author: Dominik Adamski (DominikAdamski)

Changes

-fcuda-is-device flag is not used for OpenMP offloading for AMD GPUs and it does not need to be added as clang cc1 option for OpenMP code.

This PR has the same functionality as #96909 but it doesn't introduce regression for virtual function support.


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.h (+1-1)
  • (modified) clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp (-2)
  • (modified) clang/test/Driver/amdgpu-openmp-toolchain.c (+1-1)
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index caa3786c033b5..657e681730c3a 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1010,7 +1010,7 @@ class CodeGenModule : public CodeGenTypeCache {
   bool shouldEmitRTTI(bool ForEH = false) {
     return (ForEH || getLangOpts().RTTI) && !getLangOpts().CUDAIsDevice &&
            !(getLangOpts().OpenMP && getLangOpts().OpenMPIsTargetDevice &&
-             getTriple().isNVPTX());
+             (getTriple().isNVPTX() || getTriple().isAMDGPU()));
   }
 
   /// Get the address of the RTTI descriptor for the given type.
diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index 1c0fb4babe3a5..b75d400e6ce91 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -47,8 +47,6 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
   assert(DeviceOffloadingKind == Action::OFK_OpenMP &&
          "Only OpenMP offloading kinds are supported.");
 
-  CC1Args.push_back("-fcuda-is-device");
-
   if (DriverArgs.hasArg(options::OPT_nogpulib))
     return;
 
diff --git a/clang/test/Driver/amdgpu-openmp-toolchain.c b/clang/test/Driver/amdgpu-openmp-toolchain.c
index 49af04acc4639..a153c4afb0ce8 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"{{.*}}"-fcuda-is-device"{{.*}}"-target-cpu" "gfx906"
+// CHECK: "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu"{{.*}}"-target-cpu" "gfx906"
 // CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-obj"
 // CHECK: clang-linker-wrapper{{.*}} "-o" "a.out"
 

@llvmbot
Copy link
Member

llvmbot commented Jul 16, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Dominik Adamski (DominikAdamski)

Changes

-fcuda-is-device flag is not used for OpenMP offloading for AMD GPUs and it does not need to be added as clang cc1 option for OpenMP code.

This PR has the same functionality as #96909 but it doesn't introduce regression for virtual function support.


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.h (+1-1)
  • (modified) clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp (-2)
  • (modified) clang/test/Driver/amdgpu-openmp-toolchain.c (+1-1)
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index caa3786c033b5..657e681730c3a 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1010,7 +1010,7 @@ class CodeGenModule : public CodeGenTypeCache {
   bool shouldEmitRTTI(bool ForEH = false) {
     return (ForEH || getLangOpts().RTTI) && !getLangOpts().CUDAIsDevice &&
            !(getLangOpts().OpenMP && getLangOpts().OpenMPIsTargetDevice &&
-             getTriple().isNVPTX());
+             (getTriple().isNVPTX() || getTriple().isAMDGPU()));
   }
 
   /// Get the address of the RTTI descriptor for the given type.
diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index 1c0fb4babe3a5..b75d400e6ce91 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -47,8 +47,6 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
   assert(DeviceOffloadingKind == Action::OFK_OpenMP &&
          "Only OpenMP offloading kinds are supported.");
 
-  CC1Args.push_back("-fcuda-is-device");
-
   if (DriverArgs.hasArg(options::OPT_nogpulib))
     return;
 
diff --git a/clang/test/Driver/amdgpu-openmp-toolchain.c b/clang/test/Driver/amdgpu-openmp-toolchain.c
index 49af04acc4639..a153c4afb0ce8 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"{{.*}}"-fcuda-is-device"{{.*}}"-target-cpu" "gfx906"
+// CHECK: "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu"{{.*}}"-target-cpu" "gfx906"
 // CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-obj"
 // CHECK: clang-linker-wrapper{{.*}} "-o" "a.out"
 

@DominikAdamski DominikAdamski merged commit 14c323c into llvm:main Jul 18, 2024
13 checks passed
DominikAdamski added a commit to DominikAdamski/llvm-project-upstream that referenced this pull request Jul 18, 2024
This reverts commit f2e362e.

PR: llvm#99002 causes
that -fcuda-is-device is not added for AMD GPU OpenMP.
Harini0924 pushed a commit to Harini0924/llvm-project that referenced this pull request Jul 22, 2024
-fcuda-is-device flag is not used for OpenMP offloading for AMD GPUs and
it does not need to be added as clang cc1 option for OpenMP code.

This PR has the same functionality as
llvm#96909 but it doesn't introduce
regression for virtual function support.
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
-fcuda-is-device flag is not used for OpenMP offloading for AMD GPUs and
it does not need to be added as clang cc1 option for OpenMP code.

This PR has the same functionality as
llvm#96909 but it doesn't introduce
regression for virtual function support.
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
-fcuda-is-device flag is not used for OpenMP offloading for AMD GPUs and
it does not need to be added as clang cc1 option for OpenMP code.

This PR has the same functionality as
#96909 but it doesn't introduce
regression for virtual function support.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251022
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Dec 20, 2024
-fcuda-is-device flag is not used for OpenMP offloading for AMD GPUs and
it does not need to be added as clang cc1 option for OpenMP code.

This PR has the same functionality as
llvm#96909 but it doesn't introduce
regression for virtual function support.

Change-Id: I37b2cad2214173cb1a5ffe96cdab301f49d3599e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU clang:codegen clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants