[PATCH] D122352: [OpenMP] Do not create offloading entries for internal or hidden symbols
This revision was automatically updated to reflect the committed changes. Closed by commit rG0d16c23af1da: [OpenMP] Do not create offloading entries for internal or hidden symbols (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122352/new/ https://reviews.llvm.org/D122352 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/test/OpenMP/declare_target_visibility_codegen.cpp clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp Index: clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp === --- clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp +++ clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp @@ -72,8 +72,6 @@ // DEVICE-DAG: call void // DEVICE-DAG: ret void -// HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_ADDR]]\00" -// HOST-DAG: @.omp_offloading.entry.[[C_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (i32* @[[C_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[CD_ADDR]]\00" // HOST-DAG: @.omp_offloading.entry.[[CD_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (%struct.S* @[[CD_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_CTOR]]\00" Index: clang/test/OpenMP/declare_target_visibility_codegen.cpp === --- /dev/null +++ clang/test/OpenMP/declare_target_visibility_codegen.cpp @@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix=HOST + +// expected-no-diagnostics +class C { +public: +//. +// HOST: @[[C:.+]] = internal global %class.C zeroinitializer, align 4 +// HOST: @[[X:.+]] = internal global i32 0, align 4 +// HOST: @y = hidden global i32 0 +// HOST: @z = global i32 0 +// HOST-NOT: @.omp_offloading.entry.c +// HOST-NOT: @.omp_offloading.entry.x +// HOST-NOT: @.omp_offloading.entry.y +// HOST: @.omp_offloading.entry.z + C() : x(0) {} + + int x; +}; + +static C c; +#pragma omp declare target(c) + +static int x; +#pragma omp declare target(x) + +int __attribute__((visibility("hidden"))) y; +#pragma omp declare target(y) + +int z; +#pragma omp declare target(z) Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp === --- clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -3325,6 +3325,13 @@ } break; } + + // Hidden or internal symbols on the device are not externally visible. We + // should not attempt to register them by creating an offloading entry. + if (auto *GV = dyn_cast(CE->getAddress())) +if (GV->hasLocalLinkage() || GV->hasHiddenVisibility()) + continue; + createOffloadEntry(CE->getAddress(), CE->getAddress(), CE->getVarSize().getQuantity(), Flags, CE->getLinkage()); Index: clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp === --- clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp +++ clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp @@ -72,8 +72,6 @@ // DEVICE-DAG: call void // DEVICE-DAG: ret void -// HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_ADDR]]\00" -// HOST-DAG: @.omp_offloading.entry.[[C_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (i32* @[[C_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[CD_ADDR]]\00" // HOST-DAG: @.omp_offloading.entry.[[CD_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (%struct.S* @[[CD_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8]
[PATCH] D122352: [OpenMP] Do not create offloading entries for internal or hidden symbols
JonChesterfield added a comment. Sounds good to me too, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122352/new/ https://reviews.llvm.org/D122352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122352: [OpenMP] Do not create offloading entries for internal or hidden symbols
jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122352/new/ https://reviews.llvm.org/D122352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122352: [OpenMP] Do not create offloading entries for internal or hidden symbols
jhuber6 updated this revision to Diff 417751. jhuber6 added a comment. Adding new test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122352/new/ https://reviews.llvm.org/D122352 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/test/OpenMP/declare_target_visibility_codegen.cpp clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp Index: clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp === --- clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp +++ clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp @@ -72,8 +72,6 @@ // DEVICE-DAG: call void // DEVICE-DAG: ret void -// HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_ADDR]]\00" -// HOST-DAG: @.omp_offloading.entry.[[C_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (i32* @[[C_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[CD_ADDR]]\00" // HOST-DAG: @.omp_offloading.entry.[[CD_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (%struct.S* @[[CD_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_CTOR]]\00" Index: clang/test/OpenMP/declare_target_visibility_codegen.cpp === --- /dev/null +++ clang/test/OpenMP/declare_target_visibility_codegen.cpp @@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix=HOST + +// expected-no-diagnostics +class C { +public: +//. +// HOST: @[[C:.+]] = internal global %class.C zeroinitializer, align 4 +// HOST: @[[X:.+]] = internal global i32 0, align 4 +// HOST: @y = hidden global i32 0 +// HOST: @z = global i32 0 +// HOST-NOT: @.omp_offloading.entry.c +// HOST-NOT: @.omp_offloading.entry.x +// HOST-NOT: @.omp_offloading.entry.y +// HOST: @.omp_offloading.entry.z + C() : x(0) {} + + int x; +}; + +static C c; +#pragma omp declare target(c) + +static int x; +#pragma omp declare target(x) + +int __attribute__((visibility("hidden"))) y; +#pragma omp declare target(y) + +int z; +#pragma omp declare target(z) Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp === --- clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -3323,6 +3323,13 @@ } break; } + + // Hidden or internal symbols on the device are not externally visible. We + // should not attempt to register them by creating an offloading entry. + if (auto *GV = dyn_cast(CE->getAddress())) +if (GV->hasLocalLinkage() || GV->hasHiddenVisibility()) + continue; + createOffloadEntry(CE->getAddress(), CE->getAddress(), CE->getVarSize().getQuantity(), Flags, CE->getLinkage()); Index: clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp === --- clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp +++ clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp @@ -72,8 +72,6 @@ // DEVICE-DAG: call void // DEVICE-DAG: ret void -// HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_ADDR]]\00" -// HOST-DAG: @.omp_offloading.entry.[[C_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (i32* @[[C_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[CD_ADDR]]\00" // HOST-DAG: @.omp_offloading.entry.[[CD_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (%struct.S* @[[CD_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_CTOR]]\00" Index: clang/test/OpenMP/declare_target_visibility_codegen.cpp
[PATCH] D122352: [OpenMP] Do not create offloading entries for internal or hidden symbols
jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, JonChesterfield, ronlieb, ABataev. Herald added subscribers: asavonic, guansong, yaxunl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Currently we create offloading entries to register device variables with the host. When we register a variable we will look up the symbol in the device image and map the device address to the host address. This is a problem when the symbol is declared with hidden visibility or internal linkage. This means the symbol is not accessible externally and we cannot get its address. We should still allow static variables to be declared on the device, but ew should not create an offloading entry for them so they exist independently on the host and device. Fixes #54309 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D122352 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp Index: clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp === --- clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp +++ clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp @@ -72,8 +72,6 @@ // DEVICE-DAG: call void // DEVICE-DAG: ret void -// HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_ADDR]]\00" -// HOST-DAG: @.omp_offloading.entry.[[C_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (i32* @[[C_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[CD_ADDR]]\00" // HOST-DAG: @.omp_offloading.entry.[[CD_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (%struct.S* @[[CD_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_CTOR]]\00" Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp === --- clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -3323,6 +3323,15 @@ } break; } + + // Hidden or internal symbols on the device are not externally visible. We + // should not attempt to register them by creating an offloading entry. + if (auto *GV = dyn_cast(CE->getAddress())) +if (GV->getLinkage() == llvm::GlobalValue::InternalLinkage || +GV->getLinkage() == llvm::GlobalValue::PrivateLinkage || +GV->getVisibility() == llvm::GlobalValue::HiddenVisibility) + continue; + createOffloadEntry(CE->getAddress(), CE->getAddress(), CE->getVarSize().getQuantity(), Flags, CE->getLinkage()); Index: clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp === --- clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp +++ clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp @@ -72,8 +72,6 @@ // DEVICE-DAG: call void // DEVICE-DAG: ret void -// HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_ADDR]]\00" -// HOST-DAG: @.omp_offloading.entry.[[C_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (i32* @[[C_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[CD_ADDR]]\00" // HOST-DAG: @.omp_offloading.entry.[[CD_ADDR]] = weak{{.*}} constant %struct.__tgt_offload_entry { i8* bitcast (%struct.S* @[[CD_ADDR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1 // HOST-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"[[C_CTOR]]\00" Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp === --- clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -3323,6 +3323,15 @@ } break; } + + // Hidden or internal symbols on the device are not externally visible. We +