From d081bfcd17c1c704776a0964d239f19f6acde93d Mon Sep 17 00:00:00 2001 From: Joseph Huber Date: Fri, 14 Jan 2022 17:21:46 -0500 Subject: [PATCH] [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 --- clang/lib/AST/Decl.cpp | 11 ++++++++++ clang/test/OpenMP/declare_target_codegen.cpp | 22 +++++++++++++------ ...clare_target_only_one_side_compilation.cpp | 6 ++--- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 3ef08cab967505..232376e4e05dbd 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(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 f06fd0905eaa17..6ea9f181b5cbd0 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 9bb4f6f6c6ec38..edd899281a2712 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