jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992. Herald added subscribers: guansong, yaxunl. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang.
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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D117362 Files: clang/lib/AST/Decl.cpp clang/test/OpenMP/declare_target_codegen.cpp clang/test/OpenMP/declare_target_only_one_side_compilation.cpp Index: clang/test/OpenMP/declare_target_only_one_side_compilation.cpp =================================================================== --- clang/test/OpenMP/declare_target_only_one_side_compilation.cpp +++ clang/test/OpenMP/declare_target_only_one_side_compilation.cpp @@ -57,11 +57,11 @@ // 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 Index: clang/test/OpenMP/declare_target_codegen.cpp =================================================================== --- clang/test/OpenMP/declare_target_codegen.cpp +++ clang/test/OpenMP/declare_target_codegen.cpp @@ -26,25 +26,25 @@ // 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: @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 = hidden 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*)], Index: clang/lib/AST/Decl.cpp =================================================================== --- clang/lib/AST/Decl.cpp +++ clang/lib/AST/Decl.cpp @@ -912,8 +912,11 @@ if (!isExternallyVisible(LV.getLinkage())) return LinkageInfo(LV.getLinkage(), DefaultVisibility, false); - // Mark the symbols as hidden when compiling for the device. - if (Context.getLangOpts().OpenMP && Context.getLangOpts().OpenMPIsDevice) + // Mark the symbols as hidden when compiling for the device unless it is a + // declare target definition. + const VarDecl* VD = dyn_cast<VarDecl>(D); + if (Context.getLangOpts().OpenMP && Context.getLangOpts().OpenMPIsDevice && + !(VD && OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD))) LV.mergeVisibility(HiddenVisibility, /*newExplicit=*/false); return LV;
Index: clang/test/OpenMP/declare_target_only_one_side_compilation.cpp =================================================================== --- clang/test/OpenMP/declare_target_only_one_side_compilation.cpp +++ clang/test/OpenMP/declare_target_only_one_side_compilation.cpp @@ -57,11 +57,11 @@ // 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 Index: clang/test/OpenMP/declare_target_codegen.cpp =================================================================== --- clang/test/OpenMP/declare_target_codegen.cpp +++ clang/test/OpenMP/declare_target_codegen.cpp @@ -26,25 +26,25 @@ // 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: @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 = hidden 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*)], Index: clang/lib/AST/Decl.cpp =================================================================== --- clang/lib/AST/Decl.cpp +++ clang/lib/AST/Decl.cpp @@ -912,8 +912,11 @@ if (!isExternallyVisible(LV.getLinkage())) return LinkageInfo(LV.getLinkage(), DefaultVisibility, false); - // Mark the symbols as hidden when compiling for the device. - if (Context.getLangOpts().OpenMP && Context.getLangOpts().OpenMPIsDevice) + // Mark the symbols as hidden when compiling for the device unless it is a + // declare target definition. + const VarDecl* VD = dyn_cast<VarDecl>(D); + if (Context.getLangOpts().OpenMP && Context.getLangOpts().OpenMPIsDevice && + !(VD && OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD))) LV.mergeVisibility(HiddenVisibility, /*newExplicit=*/false); return LV;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits