r338139 - [OPENMP] Static variables on device must be externally visible.

[Jonas Hahnfeld via cfe-commits]

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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits