[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.

Author: jhuber6

clang/lib/CodeGen/CodeGenModule.cpp

@@ -1391,21 +1391,23 @@ 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.
   if (Context.getLangOpts().OpenMP &&
       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())

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

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"
 //.