Hi Alexey,

from what I can see this change can't handle the case where there are static variables with the same name in multiple TUs. (The same problem exists for static CUDA kernels with -fcuda-rdc. I found that nvcc mangles the function names in this case, but didn't have time yet to prepare a similar patch for Clang.)

I think for now it would be better to emit a meaningful error instead of generating incorrect code and letting the user figure out what went wrong.

My 2 cents,
Jonas

On 2018-07-27 19:37, Alexey Bataev via cfe-commits wrote:
Author: abataev
Date: Fri Jul 27 10:37:32 2018
New Revision: 338139

URL: http://llvm.org/viewvc/llvm-project?rev=338139&view=rev
Log:
[OPENMP] Static variables on device must be externally visible.

Do not mark static variable as internal on the device as they must be
visible from the host to be mapped correctly.

Modified:
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/test/OpenMP/declare_target_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=338139&r1=338138&r2=338139&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Fri Jul 27 10:37:32 2018
@@ -9504,6 +9504,21 @@ static GVALinkage basicGVALinkageForFunc
   return GVA_DiscardableODR;
 }

+static bool isDeclareTargetToDeclaration(const Decl *VD) {
+  for (const Decl *D : VD->redecls()) {
+    if (!D->hasAttrs())
+      continue;
+    if (const auto *Attr = D->getAttr<OMPDeclareTargetDeclAttr>())
+      return Attr->getMapType() == OMPDeclareTargetDeclAttr::MT_To;
+  }
+  if (const auto *V = dyn_cast<VarDecl>(VD)) {
+    if (const VarDecl *TD = V->getTemplateInstantiationPattern())
+      return isDeclareTargetToDeclaration(TD);
+  }
+
+  return false;
+}
+
static GVALinkage adjustGVALinkageForAttributes(const ASTContext &Context, const Decl *D, GVALinkage L) {
   // See http://msdn.microsoft.com/en-us/library/xa0d9ste.aspx
@@ -9520,6 +9535,12 @@ static GVALinkage adjustGVALinkageForAtt
     // visible externally so they can be launched from host.
     if (L == GVA_DiscardableODR || L == GVA_Internal)
       return GVA_StrongODR;
+  } else if (Context.getLangOpts().OpenMP &&
Context.getLangOpts().OpenMPIsDevice &&
+             isDeclareTargetToDeclaration(D)) {
+ // Static variables must be visible externally so they can be mapped from
+    // host.
+    if (L == GVA_Internal)
+      return GVA_StrongODR;
   }
   return L;
 }

Modified: cfe/trunk/test/OpenMP/declare_target_codegen.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_codegen.cpp?rev=338139&r1=338138&r2=338139&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/declare_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_codegen.cpp Fri Jul 27 10:37:32 2018
@@ -18,12 +18,14 @@
 // CHECK-DAG: @d = global i32 0,
 // CHECK-DAG: @c = external global i32,
 // CHECK-DAG: @globals = global %struct.S zeroinitializer,
-// CHECK-DAG: @llvm.used = appending global [1 x i8*] [i8* bitcast
(void ()* @__omp_offloading__{{.+}}_globals_l[[@LINE+41]]_ctor to
i8*)], section "llvm.metadata"
+// CHECK-DAG: @{{.+}}stat = weak_odr global %struct.S zeroinitializer,
+// CHECK-DAG: @llvm.used = appending global [2 x i8*] [i8* bitcast
(void ()* @__omp_offloading__{{.+}}_globals_l[[@LINE+42]]_ctor to
i8*), i8* bitcast (void ()*
@__omp_offloading__{{.+}}_stat_l[[@LINE+43]]_ctor to i8*)], section
"llvm.metadata"

// CHECK-DAG: define {{.*}}i32 @{{.*}}{{foo|bar|baz2|baz3|FA|f_method}}{{.*}}()
 // CHECK-DAG: define {{.*}}void
@{{.*}}TemplateClass{{.*}}(%class.TemplateClass* %{{.*}})
 // CHECK-DAG: define {{.*}}i32
@{{.*}}TemplateClass{{.*}}f_method{{.*}}(%class.TemplateClass*
%{{.*}})
-// CHECK-DAG: define {{.*}}void
@__omp_offloading__{{.*}}_globals_l[[@LINE+36]]_ctor()
+// CHECK-DAG: define {{.*}}void
@__omp_offloading__{{.*}}_globals_l[[@LINE+37]]_ctor()
+// CHECK-DAG: define {{.*}}void
@__omp_offloading__{{.*}}_stat_l[[@LINE+37]]_ctor()

 #ifndef HEADER
 #define HEADER
@@ -60,6 +62,7 @@ int foo() { return 0; }
 int b = 15;
 int d;
 S globals(d);
+static S stat(d);
 #pragma omp end declare target
 int c;


Modified: cfe/trunk/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp?rev=338139&r1=338138&r2=338139&view=diff
==============================================================================
---
cfe/trunk/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
(original)
+++
cfe/trunk/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
Fri Jul 27 10:37:32 2018
@@ -15,7 +15,7 @@

 // SIMD-ONLY-NOT: {{__kmpc|__tgt}}

-// DEVICE-DAG: [[C_ADDR:.+]] = internal global i32 0,
+// DEVICE-DAG: [[C_ADDR:.+]] = weak_odr global i32 0,
 // DEVICE-DAG: [[CD_ADDR:@.+]] = global %struct.S zeroinitializer,
 // HOST-DAG: @[[C_ADDR:.+]] = internal global i32 0,
 // HOST-DAG: @[[CD_ADDR:.+]] = global %struct.S zeroinitializer,


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to