[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360342: [OpenCL] Make global ctor init function a kernel 
(authored by stulova, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61488?vs=198678=198814#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61488

Files:
  cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.h
  cfe/trunk/test/CodeGenOpenCLCXX/global_init.cl

Index: cfe/trunk/test/CodeGenOpenCLCXX/global_init.cl
===
--- cfe/trunk/test/CodeGenOpenCLCXX/global_init.cl
+++ cfe/trunk/test/CodeGenOpenCLCXX/global_init.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct S {
+  S() {}
+};
+
+S s;
+
+//CHECK: define internal spir_kernel void @_GLOBAL__sub_I_{{.*}}!kernel_arg_addr_space [[ARGMD:![0-9]+]] !kernel_arg_access_qual [[ARGMD]] !kernel_arg_type [[ARGMD]] !kernel_arg_base_type [[ARGMD]] !kernel_arg_type_qual [[ARGMD]]
+// Check that parameters are empty.
+//CHECK: [[ARGMD]] = !{}
Index: cfe/trunk/lib/CodeGen/CodeGenModule.h
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.h
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h
@@ -1315,6 +1315,19 @@
   llvm::Value *
   createOpenCLIntToSamplerConversion(const Expr *E, CodeGenFunction );
 
+  /// OpenCL v1.2 s5.6.4.6 allows the compiler to store kernel argument
+  /// information in the program executable. The argument information stored
+  /// includes the argument name, its type, the address and access qualifiers
+  /// used. This helper can be used to generate metadata for source code kernel
+  /// function as well as generated implicitly kernels. If a kernel is generated
+  /// implicitly null value has to be passed to the last two parameters,
+  /// otherwise all parameters must have valid non-null values.
+  /// \param FN is a pointer to IR function being generated.
+  /// \param FD is a pointer to function declaration if any.
+  /// \param CGF is a pointer to CodeGenFunction that generates this function.
+  void GenOpenCLArgMetadata(llvm::Function *Fn, const FunctionDecl *FD=nullptr,  
+ CodeGenFunction *CGF=nullptr);
+
   /// Get target specific null pointer.
   /// \param T is the LLVM type of the null pointer.
   /// \param QT is the clang QualType of the null pointer.
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
@@ -536,205 +536,6 @@
 "decoded_addr");
 }
 
-static void removeImageAccessQualifier(std::string& TyName) {
-  std::string ReadOnlyQual("__read_only");
-  std::string::size_type ReadOnlyPos = TyName.find(ReadOnlyQual);
-  if (ReadOnlyPos != std::string::npos)
-// "+ 1" for the space after access qualifier.
-TyName.erase(ReadOnlyPos, ReadOnlyQual.size() + 1);
-  else {
-std::string WriteOnlyQual("__write_only");
-std::string::size_type WriteOnlyPos = TyName.find(WriteOnlyQual);
-if (WriteOnlyPos != std::string::npos)
-  TyName.erase(WriteOnlyPos, WriteOnlyQual.size() + 1);
-else {
-  std::string ReadWriteQual("__read_write");
-  std::string::size_type ReadWritePos = TyName.find(ReadWriteQual);
-  if (ReadWritePos != std::string::npos)
-TyName.erase(ReadWritePos, ReadWriteQual.size() + 1);
-}
-  }
-}
-
-// Returns the address space id that should be produced to the
-// kernel_arg_addr_space metadata. This is always fixed to the ids
-// as specified in the SPIR 2.0 specification in order to differentiate
-// for example in clGetKernelArgInfo() implementation between the address
-// spaces with targets without unique mapping to the OpenCL address spaces
-// (basically all single AS CPUs).
-static unsigned ArgInfoAddressSpace(LangAS AS) {
-  switch (AS) {
-  case LangAS::opencl_global:   return 1;
-  case LangAS::opencl_constant: return 2;
-  case LangAS::opencl_local:return 3;
-  case LangAS::opencl_generic:  return 4; // Not in SPIR 2.0 specs.
-  default:
-return 0; // Assume private.
-  }
-}
-
-// OpenCL v1.2 s5.6.4.6 allows the compiler to store kernel argument
-// information in the program executable. The argument information stored
-// includes the argument name, its type, the address and access qualifiers used.
-static void GenOpenCLArgMetadata(const FunctionDecl *FD, llvm::Function *Fn,
- CodeGenModule , llvm::LLVMContext ,
- CGBuilderTy , ASTContext ) {
-  // Create MDNodes that represent the kernel arg 

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D61488#1496154 , @kpet wrote:

> The comments could use a bit more polishing but nothing that justifies making 
> another iteration IMHO. LGTM!


Sure, feel free to put up a patch if you like. :)

Thanks!


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

https://reviews.llvm.org/D61488



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

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Kévin Petit via Phabricator via cfe-commits]

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

The comments could use a bit more polishing but nothing that justifies making 
another iteration IMHO. LGTM!


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

https://reviews.llvm.org/D61488



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

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 198678.
Anastasia added a comment.

- Improved comments
- Switched to SPIR kernel


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

https://reviews.llvm.org/D61488

Files:
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenOpenCLCXX/global_init.cl

Index: test/CodeGenOpenCLCXX/global_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/global_init.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct S {
+  S() {}
+};
+
+S s;
+
+//CHECK: define internal spir_kernel void @_GLOBAL__sub_I_{{.*}}!kernel_arg_addr_space [[ARGMD:![0-9]+]] !kernel_arg_access_qual [[ARGMD]] !kernel_arg_type [[ARGMD]] !kernel_arg_base_type [[ARGMD]] !kernel_arg_type_qual [[ARGMD]]
+// Check that parameters are empty.
+//CHECK: [[ARGMD]] = !{}
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1315,6 +1315,19 @@
   llvm::Value *
   createOpenCLIntToSamplerConversion(const Expr *E, CodeGenFunction );
 
+  /// OpenCL v1.2 s5.6.4.6 allows the compiler to store kernel argument
+  /// information in the program executable. The argument information stored
+  /// includes the argument name, its type, the address and access qualifiers
+  /// used. This helper can be used to generate metadata for source code kernel
+  /// function as well as generated implicitly kernels. If a kernel is generated
+  /// implicitly null value has to be passed to the last two parameters,
+  /// otherwise all parameters must have valid non-null values.
+  /// \param FN is a pointer to IR function being generated.
+  /// \param FD is a pointer to function declaration if any.
+  /// \param CGF is a pointer to CodeGenFunction that generates this function.
+  void GenOpenCLArgMetadata(llvm::Function *Fn, const FunctionDecl *FD=nullptr,  
+ CodeGenFunction *CGF=nullptr);
+
   /// Get target specific null pointer.
   /// \param T is the LLVM type of the null pointer.
   /// \param QT is the clang QualType of the null pointer.
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1187,6 +1187,212 @@
   F->setCallingConv(static_cast(CallingConv));
 }
 
+static void removeImageAccessQualifier(std::string& TyName) {
+  std::string ReadOnlyQual("__read_only");
+  std::string::size_type ReadOnlyPos = TyName.find(ReadOnlyQual);
+  if (ReadOnlyPos != std::string::npos)
+// "+ 1" for the space after access qualifier.
+TyName.erase(ReadOnlyPos, ReadOnlyQual.size() + 1);
+  else {
+std::string WriteOnlyQual("__write_only");
+std::string::size_type WriteOnlyPos = TyName.find(WriteOnlyQual);
+if (WriteOnlyPos != std::string::npos)
+  TyName.erase(WriteOnlyPos, WriteOnlyQual.size() + 1);
+else {
+  std::string ReadWriteQual("__read_write");
+  std::string::size_type ReadWritePos = TyName.find(ReadWriteQual);
+  if (ReadWritePos != std::string::npos)
+TyName.erase(ReadWritePos, ReadWriteQual.size() + 1);
+}
+  }
+}
+
+// Returns the address space id that should be produced to the
+// kernel_arg_addr_space metadata. This is always fixed to the ids
+// as specified in the SPIR 2.0 specification in order to differentiate
+// for example in clGetKernelArgInfo() implementation between the address
+// spaces with targets without unique mapping to the OpenCL address spaces
+// (basically all single AS CPUs).
+static unsigned ArgInfoAddressSpace(LangAS AS) {
+  switch (AS) {
+  case LangAS::opencl_global:   return 1;
+  case LangAS::opencl_constant: return 2;
+  case LangAS::opencl_local:return 3;
+  case LangAS::opencl_generic:  return 4; // Not in SPIR 2.0 specs.
+  default:
+return 0; // Assume private.
+  }
+}
+
+void CodeGenModule::GenOpenCLArgMetadata(llvm::Function *Fn,
+ const FunctionDecl *FD,
+ CodeGenFunction *CGF) {
+  assert(((FD && CGF) || (!FD && !CGF)) &&
+ "Incorrect use - FD and CGF should either be both null or not!");
+  // Create MDNodes that represent the kernel arg metadata.
+  // Each MDNode is a list in the form of "key", N number of values which is
+  // the same number of values as their are kernel arguments.
+
+  const PrintingPolicy  = Context.getPrintingPolicy();
+
+  // MDNode for the kernel argument address space qualifiers.
+  SmallVector addressQuals;
+
+  // MDNode for the kernel argument access qualifiers (images only).
+  SmallVector accessQuals;
+
+  // MDNode for the kernel argument type names.
+  SmallVector argTypeNames;
+
+  // MDNode for the kernel argument base type 

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Kévin Petit via Phabricator via cfe-commits]

kpet added inline comments.



Comment at: lib/CodeGen/CGDeclCXX.cpp:583
 
+  // In OpenCL global init function should be converted to the kernel to be
+  // able to initiate its execution from the host side.

functions
should be -> must be?
to the kernel -> to a kernel



Comment at: lib/CodeGen/CGDeclCXX.cpp:584
+  // In OpenCL global init function should be converted to the kernel to be
+  // able to initiate its execution from the host side.
+  // FIXME: Some more work might be needed to handle destructors correctly.

in order that they may be launched by the host?



Comment at: lib/CodeGen/CGDeclCXX.cpp:590
+  // dynamic resource allocation on the device and program scope variables are
+  // destroyed by the runtime when program is released.
+  if (getLangOpts().OpenCL) {

Agree that global destructors aren't that attractive a feature in most 
contexts. There are quite a few runtime issues with them too. We can think 
about this later.



Comment at: lib/CodeGen/CGDeclCXX.cpp:593
+GenOpenCLArgMetadata(Fn);
+Fn->setCallingConv(llvm::CallingConv::SPIR_FUNC);
+  }

Shouldn't the calling convention be SPIR_KERNEL if you want the function to be 
host-visible?



Comment at: lib/CodeGen/CodeGenModule.h:1322
+  /// used. This helper can be used to generate metadata for source code kernel
+  /// function as well as generated implicitly kernels. If kernel is generated
+  /// implicitly null value have to be passed to the last two parameter,

kernels generated implicitly
if a kernel | when a kernel


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

https://reviews.llvm.org/D61488



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

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CodeGenModule.cpp:1231
+  assert(((FD && CGF) || (!FD && !CGF)) &&
+ "Incorrect use - FD and CFG should either be both null or not!");
+  // Create MDNodes that represent the kernel arg metadata.

CFG -> CGF


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

https://reviews.llvm.org/D61488



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

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: kpet, rjmccall.
Herald added subscribers: ebevhan, yaxunl.

We need to be able to enqueue internal function that initializes global 
constructors on the host side. Therefore it has to be converted to a kernel.

Note, supporting destruction would need some more work. However, it seems 
global destruction has little meaning without any dynamic resource allocation 
on the device and program scope variables are destroyed by the runtime when 
program is released.


https://reviews.llvm.org/D61488

Files:
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenOpenCLCXX/global_init.cl

Index: test/CodeGenOpenCLCXX/global_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/global_init.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct S {
+  S() {}
+};
+
+S s;
+
+//CHECK: define internal spir_func void @_GLOBAL__sub_I_{{.*}}!kernel_arg_addr_space [[ARGMD:![0-9]+]] !kernel_arg_access_qual [[ARGMD]] !kernel_arg_type [[ARGMD]] !kernel_arg_base_type [[ARGMD]] !kernel_arg_type_qual [[ARGMD]]
+// Check that parameters are empty.
+//CHECK: [[ARGMD]] = !{}
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1315,6 +1315,19 @@
   llvm::Value *
   createOpenCLIntToSamplerConversion(const Expr *E, CodeGenFunction );
 
+  /// OpenCL v1.2 s5.6.4.6 allows the compiler to store kernel argument
+  /// information in the program executable. The argument information stored
+  /// includes the argument name, its type, the address and access qualifiers
+  /// used. This helper can be used to generate metadata for source code kernel
+  /// function as well as generated implicitly kernels. If kernel is generated
+  /// implicitly null value have to be passed to the last two parameter,
+  /// otherwise all parameters must have valid non-null values.
+  /// \param FN is a pointer to IR function being generated.
+  /// \param FD is a pointer to function declaration if any.
+  /// \param CGF is a pointer to CodeGenFunction that generates this function.
+  void GenOpenCLArgMetadata(llvm::Function *Fn, const FunctionDecl *FD=nullptr,  
+ CodeGenFunction *CGF=nullptr);
+
   /// Get target specific null pointer.
   /// \param T is the LLVM type of the null pointer.
   /// \param QT is the clang QualType of the null pointer.
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1187,6 +1187,212 @@
   F->setCallingConv(static_cast(CallingConv));
 }
 
+static void removeImageAccessQualifier(std::string& TyName) {
+  std::string ReadOnlyQual("__read_only");
+  std::string::size_type ReadOnlyPos = TyName.find(ReadOnlyQual);
+  if (ReadOnlyPos != std::string::npos)
+// "+ 1" for the space after access qualifier.
+TyName.erase(ReadOnlyPos, ReadOnlyQual.size() + 1);
+  else {
+std::string WriteOnlyQual("__write_only");
+std::string::size_type WriteOnlyPos = TyName.find(WriteOnlyQual);
+if (WriteOnlyPos != std::string::npos)
+  TyName.erase(WriteOnlyPos, WriteOnlyQual.size() + 1);
+else {
+  std::string ReadWriteQual("__read_write");
+  std::string::size_type ReadWritePos = TyName.find(ReadWriteQual);
+  if (ReadWritePos != std::string::npos)
+TyName.erase(ReadWritePos, ReadWriteQual.size() + 1);
+}
+  }
+}
+
+// Returns the address space id that should be produced to the
+// kernel_arg_addr_space metadata. This is always fixed to the ids
+// as specified in the SPIR 2.0 specification in order to differentiate
+// for example in clGetKernelArgInfo() implementation between the address
+// spaces with targets without unique mapping to the OpenCL address spaces
+// (basically all single AS CPUs).
+static unsigned ArgInfoAddressSpace(LangAS AS) {
+  switch (AS) {
+  case LangAS::opencl_global:   return 1;
+  case LangAS::opencl_constant: return 2;
+  case LangAS::opencl_local:return 3;
+  case LangAS::opencl_generic:  return 4; // Not in SPIR 2.0 specs.
+  default:
+return 0; // Assume private.
+  }
+}
+
+void CodeGenModule::GenOpenCLArgMetadata(llvm::Function *Fn,
+ const FunctionDecl *FD,
+ CodeGenFunction *CGF) {
+  assert(((FD && CGF) || (!FD && !CGF)) &&
+ "Incorrect use - FD and CFG should either be both null or not!");
+  // Create MDNodes that represent the kernel arg metadata.
+  // Each MDNode is a list in the form of "key", N number of values which is
+  // the same number of values as their are kernel arguments.
+
+  const PrintingPolicy  =