[clang] d081bfc - [OpenMP] Remove hidden visibility for declare target variables

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-01-18T12:53:20-05:00
New Revision: d081bfcd17c1c704776a0964d239f19f6acde93d

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

LOG: [OpenMP] Remove hidden visibility for declare target variables

This patch changes the visiblity of variables declared within a declare
target directive. Variable declarations within a declare target
directive need to be externally visible to the plugin for initialization
or reading. Previously this would cause runtime errors where the named
global could not be found because it was not included in the symbol
table.

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D117362

Added: 
    

Modified: 
    clang/lib/AST/Decl.cpp
    clang/test/OpenMP/declare_target_codegen.cpp
    clang/test/OpenMP/declare_target_only_one_side_compilation.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 3ef08cab96750..232376e4e05db 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -786,6 +786,11 @@ LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
     //
     // Note that we don't want to make the variable non-external
     // because of this, but unique-external linkage suits us.
+
+    // We need variables inside OpenMP declare target directives to be visible.
+    if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Var))
+      return LinkageInfo::external();
+
     if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var) &&
         !IgnoreVarTypeLinkage) {
       LinkageInfo TypeLV = getLVForType(*Var->getType(), computation);
@@ -1069,6 +1074,12 @@ LinkageComputer::getLVForClassMember(const NamedDecl *D,
 
   // Finally, merge in information from the class.
   LV.mergeMaybeWithVisibility(classLV, considerClassVisibility);
+
+  // We need variables inside OpenMP declare target directives to be visible.
+  if (const VarDecl *VD = dyn_cast<VarDecl>(D))
+    if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD))
+      return LinkageInfo(LV.getLinkage(), DefaultVisibility, false);
+
   return LV;
 }
 

diff  --git a/clang/test/OpenMP/declare_target_codegen.cpp b/clang/test/OpenMP/declare_target_codegen.cpp
index f06fd0905eaa1..6ea9f181b5cbd 100644
--- a/clang/test/OpenMP/declare_target_codegen.cpp
+++ b/clang/test/OpenMP/declare_target_codegen.cpp
@@ -26,25 +26,26 @@
 // CHECK-NOT: define {{.*}}{{baz1|baz4|maini1|Base|virtual_}}
 // CHECK-DAG: Bake
 // CHECK-NOT: @{{hhh|ggg|fff|eee}} =
-// CHECK-DAG: @flag = hidden global i8 undef,
+// CHECK-DAG: @flag = global i8 undef,
 // CHECK-DAG: @aaa = external global i32,
-// CHECK-DAG: @bbb ={{ hidden | }}global i32 0,
+// CHECK-DAG: @bbb = global i32 0,
 // CHECK-DAG: weak constant %struct.__tgt_offload_entry { i8* bitcast (i32* @bbb to i8*),
 // CHECK-DAG: @ccc = external global i32,
-// CHECK-DAG: @ddd ={{ hidden | }}global i32 0,
+// CHECK-DAG: @ddd = global i32 0,
 // CHECK-DAG: @hhh_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @ggg_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @fff_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @eee_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @{{.*}}maini1{{.*}}aaa = internal global i64 23,
 // CHECK-DAG: @pair = {{.*}}addrspace(3) global %struct.PAIR undef
-// CHECK-DAG: @b ={{ hidden | }}global i32 15,
-// CHECK-DAG: @d ={{ hidden | }}global i32 0,
+// CHECK-DAG: @_ZN2SS3SSSE = global i32 1,
+// CHECK-DAG: @b = global i32 15,
+// CHECK-DAG: @d = global i32 0,
 // CHECK-DAG: @c = external global i32,
-// CHECK-DAG: @globals ={{ hidden | }}global %struct.S zeroinitializer,
+// CHECK-DAG: @globals = global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT:@.+stat]] = internal global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT_REF:@.+]] = internal constant %struct.S* [[STAT]]
-// CHECK-DAG: @out_decl_target ={{ hidden | }}global i32 0,
+// CHECK-DAG: @out_decl_target = global i32 0,
 // CHECK-DAG: @llvm.used = appending global [2 x i8*] [i8* bitcast (void ()* @__omp_offloading__{{.+}}_globals_l[[@LINE+84]]_ctor to i8*), i8* bitcast (void ()* @__omp_offloading__{{.+}}_stat_l[[@LINE+85]]_ctor to i8*)],
 // CHECK-DAG: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (%struct.S** [[STAT_REF]] to i8*)],
 
@@ -283,4 +284,11 @@ void new_bar1() {
   X->emitted();
 }
 #pragma omp end declare target
+
+struct SS {
+#pragma omp declare target
+  static int SSS;
+#pragma omp end declare target
+};
+int SS::SSS = 1;
 #endif

diff  --git a/clang/test/OpenMP/declare_target_only_one_side_compilation.cpp b/clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
index 9bb4f6f6c6ec3..edd899281a271 100644
--- a/clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
+++ b/clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
@@ -57,11 +57,11 @@ static int GY;
 
 // TODO: It is odd, probably wrong, that we don't mangle all variables.
 
-// DEVICE-DAG: @G1 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G1 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G2 = internal {{.*}}global i32 0, align 4
-// DEVICE-DAG: @G3 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G3 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G4 = internal {{.*}}global i32 0, align 4
-// DEVICE-DAG: @G5 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G5 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G6 = internal {{.*}}global i32 0, align 4
 // DEVICE-NOT: ref
 // DEVICE-NOT: llvm.used