subject:"\[PATCH\] D123353\: \[CUDA\]\[HIP\] Externalize kernels in anonymous name space"

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

2022-04-10 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rG4ea1d435099f: [CUDA][HIP] Externalize kernels in anonymous 
name space (authored by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D123353?vs=421392=421820#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123353/new/

https://reviews.llvm.org/D123353

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/kernel-in-anon-ns.cu

Index: clang/test/CodeGenCUDA/kernel-in-anon-ns.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/kernel-in-anon-ns.cu
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -cuid=abc \
+// RUN:   -aux-triple x86_64-unknown-linux-gnu -std=c++11 -fgpu-rdc \
+// RUN:   -emit-llvm -o - -x hip %s > %t.dev
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -cuid=abc \
+// RUN:   -aux-triple amdgcn-amd-amdhsa -std=c++11 -fgpu-rdc \
+// RUN:   -emit-llvm -o - -x hip %s > %t.host
+
+// RUN: cat %t.dev %t.host | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// CHECK: define weak_odr {{.*}}void @[[KERN:_ZN12_GLOBAL__N_16kernelEv\.anon\.b04fd23c98500190]](
+// CHECK: @[[STR:.*]] = {{.*}} c"[[KERN]]\00"
+// CHECK: call i32 @__hipRegisterFunction({{.*}}@[[STR]]
+
+namespace {
+__global__ void kernel() {
+}
+}
+
+void test() {
+  kernel<<<1, 1>>>();
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1457,9 +1457,10 @@
TBAAAccessInfo *TBAAInfo = nullptr);
   bool stopAutoInit();
 
-  /// Print the postfix for externalized static variable for single source
-  /// offloading languages CUDA and HIP.
-  void printPostfixForExternalizedStaticVar(llvm::raw_ostream ) const;
+  /// Print the postfix for externalized static variable or kernels for single
+  /// source offloading languages CUDA and HIP.
+  void printPostfixForExternalizedDecl(llvm::raw_ostream ,
+   const Decl *D) const;
 
 private:
   llvm::Constant *GetOrCreateLLVMFunction(
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1376,10 +1376,10 @@
 }
 
   // Make unique name for device side static file-scope variable for HIP.
-  if (CGM.getContext().shouldExternalizeStaticVar(ND) &&
+  if (CGM.getContext().shouldExternalize(ND) &&
   CGM.getLangOpts().GPURelocatableDeviceCode &&
   CGM.getLangOpts().CUDAIsDevice && !CGM.getLangOpts().CUID.empty())
-CGM.printPostfixForExternalizedStaticVar(Out);
+CGM.printPostfixForExternalizedDecl(Out, ND);
   return std::string(Out.str());
 }
 
@@ -1446,8 +1446,7 @@
   // static device variable depends on whether the variable is referenced by
   // a host or device host function. Therefore the mangled name cannot be
   // cached.
-  if (!LangOpts.CUDAIsDevice ||
-  !getContext().mayExternalizeStaticVar(GD.getDecl())) {
+  if (!LangOpts.CUDAIsDevice || !getContext().mayExternalize(GD.getDecl())) {
 auto FoundName = MangledDeclNames.find(CanonicalGD);
 if (FoundName != MangledDeclNames.end())
   return FoundName->second;
@@ -1467,7 +1466,7 @@
   // directly between host- and device-compilations, the host- and
   // device-mangling in host compilation could help catching certain ones.
   assert(!isa(ND) || !ND->hasAttr() ||
- getLangOpts().CUDAIsDevice ||
+ getContext().shouldExternalize(ND) || getLangOpts().CUDAIsDevice ||
  (getContext().getAuxTargetInfo() &&
   (getContext().getAuxTargetInfo()->getCXXABI() !=
getContext().getTargetInfo().getCXXABI())) ||
@@ -6772,7 +6771,8 @@
   return false;
 }
 
-void CodeGenModule::printPostfixForExternalizedStaticVar(
-llvm::raw_ostream ) const {
-  OS << "__static__" << getContext().getCUIDHash();
+void CodeGenModule::printPostfixForExternalizedDecl(llvm::raw_ostream ,
+const Decl *D) const {
+  OS << (isa(D) ? "__static__" : ".anon.")
+ << getContext().getCUIDHash();
 }
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -281,13 +281,13 @@
 DeviceSideName = std::string(ND->getIdentifier()->getName());
 
   // Make unique name for device side static file-scope variable for HIP.
-  if (CGM.getContext().shouldExternalizeStaticVar(ND) &&
+

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

2022-04-09 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: clang/test/CodeGenCUDA/kernel-in-anon-ns.cu:13
+
+// CHECK: define weak_odr {{.*}}void 
@[[KERN:_ZN12_GLOBAL__N_16kernelEv\.anon\..*]](
+// CHECK: @[[STR:.*]] = {{.*}} c"[[KERN]]\00"

tra wrote:
> yaxunl wrote:
> > tra wrote:
> > > Will the externalized names be uniquified as well?
> > > 
> > > E.g. if we compile with -fgpu-rdc, we do want the kernels to be 
> > > externally visible, but we also don't want the names to clash if we have 
> > > two TUs having the same external name for them.
> > Yes, the kernel name is uniquified with a hash of the source path and 
> > compile options.
> Then we should probably include the unique suffix in the CHECK line. The RUN 
> lines already provide specific cuid, so the fuffix will always be the same.
will do when committing


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123353/new/

https://reviews.llvm.org/D123353

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

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

2022-04-08 Thread Artem Belevich via Phabricator via cfe-commits

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM overall.




Comment at: clang/test/CodeGenCUDA/kernel-in-anon-ns.cu:13
+
+// CHECK: define weak_odr {{.*}}void 
@[[KERN:_ZN12_GLOBAL__N_16kernelEv\.anon\..*]](
+// CHECK: @[[STR:.*]] = {{.*}} c"[[KERN]]\00"

yaxunl wrote:
> tra wrote:
> > Will the externalized names be uniquified as well?
> > 
> > E.g. if we compile with -fgpu-rdc, we do want the kernels to be externally 
> > visible, but we also don't want the names to clash if we have two TUs 
> > having the same external name for them.
> Yes, the kernel name is uniquified with a hash of the source path and compile 
> options.
Then we should probably include the unique suffix in the CHECK line. The RUN 
lines already provide specific cuid, so the fuffix will always be the same.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123353/new/

https://reviews.llvm.org/D123353

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

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

2022-04-08 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: clang/test/CodeGenCUDA/kernel-in-anon-ns.cu:13
+
+// CHECK: define weak_odr {{.*}}void 
@[[KERN:_ZN12_GLOBAL__N_16kernelEv\.anon\..*]](
+// CHECK: @[[STR:.*]] = {{.*}} c"[[KERN]]\00"

tra wrote:
> Will the externalized names be uniquified as well?
> 
> E.g. if we compile with -fgpu-rdc, we do want the kernels to be externally 
> visible, but we also don't want the names to clash if we have two TUs having 
> the same external name for them.
Yes, the kernel name is uniquified with a hash of the source path and compile 
options.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123353/new/

https://reviews.llvm.org/D123353

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

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

2022-04-08 Thread Artem Belevich via Phabricator via cfe-commits

tra added inline comments.



Comment at: clang/test/CodeGenCUDA/kernel-in-anon-ns.cu:13
+
+// CHECK: define weak_odr {{.*}}void 
@[[KERN:_ZN12_GLOBAL__N_16kernelEv\.anon\..*]](
+// CHECK: @[[STR:.*]] = {{.*}} c"[[KERN]]\00"

Will the externalized names be uniquified as well?

E.g. if we compile with -fgpu-rdc, we do want the kernels to be externally 
visible, but we also don't want the names to clash if we have two TUs having 
the same external name for them.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123353/new/

https://reviews.llvm.org/D123353

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

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

2022-04-07 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a project: All.
yaxunl requested review of this revision.

kernels in anonymous name space needs to have unique name
to avoid duplicate symbols.

Fixes: https://github.com/llvm/llvm-project/issues/54560


https://reviews.llvm.org/D123353

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/kernel-in-anon-ns.cu

Index: clang/test/CodeGenCUDA/kernel-in-anon-ns.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/kernel-in-anon-ns.cu
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -cuid=abc \
+// RUN:   -aux-triple x86_64-unknown-linux-gnu -std=c++11 -fgpu-rdc \
+// RUN:   -emit-llvm -o - -x hip %s > %t.dev
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -cuid=abc \
+// RUN:   -aux-triple amdgcn-amd-amdhsa -std=c++11 -fgpu-rdc \
+// RUN:   -emit-llvm -o - -x hip %s > %t.host
+
+// RUN: cat %t.dev %t.host | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// CHECK: define weak_odr {{.*}}void @[[KERN:_ZN12_GLOBAL__N_16kernelEv\.anon\..*]](
+// CHECK: @[[STR:.*]] = {{.*}} c"[[KERN]]\00"
+// CHECK: call i32 @__hipRegisterFunction({{.*}}@[[STR]] 
+
+namespace {
+__global__ void kernel() {
+}
+}
+
+void test() {
+  kernel<<<1, 1>>>();
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1457,9 +1457,10 @@
TBAAAccessInfo *TBAAInfo = nullptr);
   bool stopAutoInit();
 
-  /// Print the postfix for externalized static variable for single source
-  /// offloading languages CUDA and HIP.
-  void printPostfixForExternalizedStaticVar(llvm::raw_ostream ) const;
+  /// Print the postfix for externalized static variable or kernels for single
+  /// source offloading languages CUDA and HIP.
+  void printPostfixForExternalizedDecl(llvm::raw_ostream ,
+   const Decl *D) const;
 
   /// Helper functions for generating a NoLoop kernel
   /// For a captured statement, get the single For statement, if it exists,
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1382,10 +1382,10 @@
 }
 
   // Make unique name for device side static file-scope variable for HIP.
-  if (CGM.getContext().shouldExternalizeStaticVar(ND) &&
+  if (CGM.getContext().shouldExternalize(ND) &&
   CGM.getLangOpts().GPURelocatableDeviceCode &&
   CGM.getLangOpts().CUDAIsDevice && !CGM.getLangOpts().CUID.empty())
-CGM.printPostfixForExternalizedStaticVar(Out);
+CGM.printPostfixForExternalizedDecl(Out, ND);
   return std::string(Out.str());
 }
 
@@ -1452,8 +1452,7 @@
   // static device variable depends on whether the variable is referenced by
   // a host or device host function. Therefore the mangled name cannot be
   // cached.
-  if (!LangOpts.CUDAIsDevice ||
-  !getContext().mayExternalizeStaticVar(GD.getDecl())) {
+  if (!LangOpts.CUDAIsDevice || !getContext().mayExternalize(GD.getDecl())) {
 auto FoundName = MangledDeclNames.find(CanonicalGD);
 if (FoundName != MangledDeclNames.end())
   return FoundName->second;
@@ -1473,7 +1472,7 @@
   // directly between host- and device-compilations, the host- and
   // device-mangling in host compilation could help catching certain ones.
   assert(!isa(ND) || !ND->hasAttr() ||
- getLangOpts().CUDAIsDevice ||
+ getContext().shouldExternalize(ND) || getLangOpts().CUDAIsDevice ||
  (getContext().getAuxTargetInfo() &&
   (getContext().getAuxTargetInfo()->getCXXABI() !=
getContext().getTargetInfo().getCXXABI())) ||
@@ -6798,9 +6797,10 @@
   return false;
 }
 
-void CodeGenModule::printPostfixForExternalizedStaticVar(
-llvm::raw_ostream ) const {
-  OS << "__static__" << getContext().getCUIDHash();
+void CodeGenModule::printPostfixForExternalizedDecl(llvm::raw_ostream ,
+const Decl *D) const {
+  OS << (isa(D) ? "__static__" : ".anon.")
+ << getContext().getCUIDHash();
 }
 
 namespace {
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -281,13 +281,13 @@
 DeviceSideName = std::string(ND->getIdentifier()->getName());
 
   // Make unique name for device side static file-scope variable for HIP.
-  if (CGM.getContext().shouldExternalizeStaticVar(ND) &&
+  if (CGM.getContext().shouldExternalize(ND) &&
   CGM.getLangOpts().GPURelocatableDeviceCode &&
   !CGM.getLangOpts().CUID.empty()) {

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

[PATCH] D123353: [CUDA][HIP] Externalize kernels in anonymous name space

6 matches

Site Navigation

Mail list logo

Footer information