[clang] 85feb93 - [OpenMP] Fix setting visibility on declare target variables

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2023-10-09T07:56:43-05:00
New Revision: 85feb9347f77859a877e767692e1c11d00cf6ffd

URL: https://github.com/llvm/llvm-project/commit/85feb9347f77859a877e767692e1c11d00cf6ffd
DIFF: https://github.com/llvm/llvm-project/commit/85feb9347f77859a877e767692e1c11d00cf6ffd.diff

LOG: [OpenMP] Fix setting visibility on declare target variables

Summary:
A previous patch changed the logic to force external visibliity on
declare target variables. This is because they need to be exported in
the dynamic symbol table to be usable as the standard depicts. However,
the logic was always setting the visibility to `protected`, which would
override some symbols. For example, when calling `libc` functions for
CPU offloading. This patch changes the logic to only fire if the
variable has hidden visibliity to start with.

Added: 
    

Modified: 
    clang/lib/CodeGen/CodeGenModule.cpp
    clang/test/OpenMP/declare_target_codegen.cpp
    clang/test/OpenMP/declare_target_constexpr_codegen.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cae9dd93bc55921..d6ab7b3567b9b03 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1391,6 +1391,10 @@ void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
   if (!D)
     return;
 
+  // Set visibility for definitions, and for declarations if requested globally
+  // or set explicitly.
+  LinkageInfo LV = D->getLinkageAndVisibility();
+
   // OpenMP declare target variables must be visible to the host so they can
   // be registered. We require protected visibility unless the variable has
   // the DT_nohost modifier and does not need to be registered.
@@ -1398,14 +1402,12 @@ void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
       Context.getLangOpts().OpenMPIsTargetDevice && isa<VarDecl>(D) &&
       D->hasAttr<OMPDeclareTargetDeclAttr>() &&
       D->getAttr<OMPDeclareTargetDeclAttr>()->getDevType() !=
-          OMPDeclareTargetDeclAttr::DT_NoHost) {
+          OMPDeclareTargetDeclAttr::DT_NoHost &&
+      LV.getVisibility() == HiddenVisibility) {
     GV->setVisibility(llvm::GlobalValue::ProtectedVisibility);
     return;
   }
 
-  // Set visibility for definitions, and for declarations if requested globally
-  // or set explicitly.
-  LinkageInfo LV = D->getLinkageAndVisibility();
   if (GV->hasDLLExportStorageClass() || GV->hasDLLImportStorageClass()) {
     // Reject incompatible dlllstorage and visibility annotations.
     if (!LV.isVisibilityExplicit())

diff  --git a/clang/test/OpenMP/declare_target_codegen.cpp b/clang/test/OpenMP/declare_target_codegen.cpp
index 225695feae95151..71c742198af6bff 100644
--- a/clang/test/OpenMP/declare_target_codegen.cpp
+++ b/clang/test/OpenMP/declare_target_codegen.cpp
@@ -31,7 +31,7 @@
 // CHECK-DAG: @dy = {{protected | }}global i32 0,
 // CHECK-DAG: @bbb = {{protected | }}global i32 0,
 // CHECK-DAG: weak constant %struct.__tgt_offload_entry { ptr @bbb,
-// CHECK-DAG: @ccc = external {{protected | }}global i32,
+// CHECK-DAG: @ccc = external global i32,
 // CHECK-DAG: @ddd = {{protected | }}global i32 0,
 // CHECK-DAG: @hhh_decl_tgt_ref_ptr = weak global ptr null
 // CHECK-DAG: @ggg_decl_tgt_ref_ptr = weak global ptr null

diff  --git a/clang/test/OpenMP/declare_target_constexpr_codegen.cpp b/clang/test/OpenMP/declare_target_constexpr_codegen.cpp
index 2b256cd6a4c7f09..0acd98129394b8a 100644
--- a/clang/test/OpenMP/declare_target_constexpr_codegen.cpp
+++ b/clang/test/OpenMP/declare_target_constexpr_codegen.cpp
@@ -16,7 +16,7 @@ class A {
 public:
   static constexpr double pi = 3.141592653589793116;
 //.
-// CHECK: @_ZN1A2piE = linkonce_odr protected constant double 0x400921FB54442D18, comdat, align 8
+// CHECK: @_ZN1A2piE = linkonce_odr constant double 0x400921FB54442D18, comdat, align 8
 // CHECK: @_ZL9anotherPi = internal constant double 3.140000e+00, align 8
 // CHECK: @llvm.compiler.used = appending global [2 x ptr] [ptr @"__ZN1A2piE$ref", ptr @"__ZL9anotherPi$ref"], section "llvm.metadata"
 //.