[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Ilia Diachkov via Phabricator via cfe-commits]

iliya-diyachkov added inline comments.



Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:100-112
+llvm::Type *CGOpenCLRuntime::getSamplerType(const Type *T) {
+  if (!SamplerTy) {
+if (llvm::Type *TransTy = CGM.getTargetCodeGenInfo().getOpenCLType(
+CGM, CGM.getContext().OCLSamplerTy.getTypePtr()))
+  SamplerTy = TransTy;
+else
+  SamplerTy = llvm::PointerType::get(

Perhaps use early exits like this, or even removing 'else' clause.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D130768: [OpenCL][SPIR-V] Add test for extern functions with a pointer

[Ilia Diachkov via Phabricator via cfe-commits]

iliya-diyachkov accepted this revision.
iliya-diyachkov added a comment.
This revision is now accepted and ready to land.

Looks good to me.


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

https://reviews.llvm.org/D130768

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

[PATCH] D130766: [SPIR-V] Disable opaque pointers in relese 15

[Ilia Diachkov via Phabricator via cfe-commits]

iliya-diyachkov added a comment.

Looks good to me.


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

https://reviews.llvm.org/D130766

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

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

[Ilia Diachkov via Phabricator via cfe-commits]

iliya-diyachkov added a comment.

In D127579#3585411 , @Anastasia wrote:

> Also is there anything that we don't need from the metadata that are 
> currently emitted for the use case i.e. for example argument names? This 
> could reduce the number of metadata emitted...

Sure, I think we don't need all info. As I mentioned in TODO I'm going to leave 
only type info and remove other metadata emission. For this purpose I'll 
separate a new function (that emits the type info) from GenOpenCLArgMetadata.

In D127579#3586472 , @jcranmer-intel 
wrote:

> I haven't yet tested whether or not this patch is sufficient to support all 
> of the use cases I need for type scavenging for SPIR-V, but one of the things 
> I do see is that there's no support for return type information, which may be 
> a bit of an issue.

I think the return type information can be added in the next version of the 
patch, however it should be attached to function declarations (not definitions 
as it's done now), right? Do you think, declarations also require information 
about argument types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127579

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

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

[Ilia Diachkov via Phabricator via cfe-commits]

iliya-diyachkov created this revision.
iliya-diyachkov added reviewers: Anastasia, bader, svenvh.
Herald added a subscriber: Naghasan.
Herald added a project: All.
iliya-diyachkov requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

It's a draft for the patch we discussed in the thread of opaque pointer support 
in the SPIR-V translator 
. The final 
decision on whether we will use this approach has not yet been made, so the 
patch is uploaded for convenience and concertizing of the discussion.

The types are already in metadata for kernel functions by default. Alexey Bader 
shared the code example  with non-kernel 
functions which lost the info. The patch fixes this case. To enable the feature 
you need to pass `-cl-extra-ptr-info` to clang.

TODO:

- emit only ptr type info metadata if EmitOpenCLExtraPtrInfo is enabled,
- maybe add the same info to function declarations,
- maybe add the same options to other languages (SYCL, HLSL ...) or make one 
common option for all languages.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127579

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3447,6 +3447,7 @@
   options::OPT_cl_single_precision_constant,
   options::OPT_cl_finite_math_only,
   options::OPT_cl_kernel_arg_info,
+  options::OPT_cl_extra_ptr_info,
   options::OPT_cl_unsafe_math_optimizations,
   options::OPT_cl_fast_relaxed_math,
   options::OPT_cl_mad_enable,
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -600,8 +600,11 @@
 void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
llvm::Function *Fn)
 {
-  if (!FD->hasAttr())
+  if (!FD->hasAttr()) {
+if (CGM.getCodeGenOpts().EmitOpenCLExtraPtrInfo)
+  CGM.GenOpenCLArgMetadata(Fn, FD, this);
 return;
+  }
 
   llvm::LLVMContext  = getLLVMContext();
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -896,6 +896,9 @@
   HelpText<"OpenCL only. Enable or disable OpenCL extensions/optional 
features. The argument is a comma-separated "
"sequence of one or more extension names, each prefixed by '+' or 
'-'.">,
   MarshallingInfoStringVector>;
+def cl_extra_ptr_info : Flag<["-"], "cl-extra-ptr-info">, Group, 
Flags<[CC1Option]>,
+  HelpText<"OpenCL only. Generate type hints for pointers.">,
+  MarshallingInfoFlag>;
 
 def client__name : JoinedOrSeparate<["-"], "client_name">;
 def combine : Flag<["-", "--"], "combine">, Flags<[NoXarchOption, 
Unsupported]>;
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -87,6 +87,7 @@
 CODEGENOPT(EmitGcovArcs  , 1, 0) ///< Emit coverage data files, aka. GCDA.
 CODEGENOPT(EmitGcovNotes , 1, 0) ///< Emit coverage "notes" files, aka 
GCNO.
 CODEGENOPT(EmitOpenCLArgMetadata , 1, 0) ///< Emit OpenCL kernel arg metadata.
+CODEGENOPT(EmitOpenCLExtraPtrInfo , 1, 0) ///< Emit OpenCL extra pointer info.
 CODEGENOPT(EmulatedTLS   , 1, 0) ///< Set by default or 
-f[no-]emulated-tls.
 CODEGENOPT(ExplicitEmulatedTLS , 1, 0) ///< Set if -f[no-]emulated-tls is used.
 /// Embed Bitcode mode (off/all/bitcode/marker).


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3447,6 +3447,7 @@
   options::OPT_cl_single_precision_constant,
   options::OPT_cl_finite_math_only,
   options::OPT_cl_kernel_arg_info,
+  options::OPT_cl_extra_ptr_info,
   options::OPT_cl_unsafe_math_optimizations,
   options::OPT_cl_fast_relaxed_math,
   options::OPT_cl_mad_enable,
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -600,8 +600,11 @@
 void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
llvm::Function *Fn)
 {
-  if (!FD->hasAttr())
+  if (!FD->hasAttr()) {
+if 

[PATCH] D125679: [Clang] Added options for integrated backend only used for SPIR-V for now

[Ilia Diachkov via Phabricator via cfe-commits]

iliya-diyachkov added a comment.

I'm not an expert in clang, but overall the patch looks good to me.




Comment at: clang/docs/UsersManual.rst:3640
+Clang also supports integrated generation of SPIR-V without use of 
``llvm-spirv``
+tool as an exerimental feature when ``-fintegrated-objemitter`` flag is passed 
in
+the command line.

Please fix typo in "exerimental".



Comment at: clang/include/clang/Driver/ToolChain.h:392
+
+  /// IsIntegratedBackendSupported - Does this tool chain supports
+  /// -fintegrated-objemitter.

Probably change "supports" to "support".



Comment at: clang/include/clang/Driver/ToolChain.h:396
+
+  /// IsNonIntegratedBackendSupported - Does this tool chain supports
+  /// -fno-integrated-objemitter.

The same issue.


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

https://reviews.llvm.org/D125679

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

[PATCH] D124776: [SPIR-V] Allow setting SPIR-V version via target triple

[Ilia Diachkov via Phabricator via cfe-commits]

iliya-diyachkov added a comment.

@Anastasia, when are you going to add support for v1.5? We would use it in the 
SPIR-V backend.


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

https://reviews.llvm.org/D124776

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