Skip to content

Commit

Permalink
[OpenMP] Remove hidden visibility for declare target variables
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jhuber6 committed Jan 18, 2022
1 parent dcb83b2 commit d081bfc
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 10 deletions.
11 changes: 11 additions & 0 deletions clang/lib/AST/Decl.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
22 changes: 15 additions & 7 deletions clang/test/OpenMP/declare_target_codegen.cpp
Expand Up @@ -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*)],

Expand Down Expand Up @@ -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
Expand Up @@ -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
Expand Down

0 comments on commit d081bfc

Please sign in to comment.