[PATCH] D110089: [CUDA] Implement experimental support for texture lookups.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/lib/Headers/__clang_cuda_texture_intrinsics.h:41
+
+namespace {
+
----------------
jlebar wrote:
> jlebar wrote:
> > what are you trying to accomplish with an anon ns inside a header?
> I know you wrote it in the commit message, but this file could really use comments, otherwise I'm afraid you are going to be the only human being on the planet who can edit this...
> 
> For starters, it seems that the purpose of this file is to define the __nv_tex_surf_handler "function" -- is that right?
> what are you trying to accomplish with an anon ns inside a header?

I wanted to give all functions internal linkage, so they do not pollute visible symbols. Without that and with numeric tag IDs not being stable, we could end up having ODR issues in code compiled with `-fgpu-rdc` by different clang versions.

I've moved all defined functions into `namespace __cuda_tex`, so I don't have to use an extra prefix on all the types the header creates.

> For starters, it seems that the purpose of this file is to define the __nv_tex_surf_handler "function" -- is that right?

Yes.  I've added a comment at the top of the doc.




================
Comment at: clang/lib/Headers/__clang_cuda_texture_intrinsics.h:91
+template <class __T> struct __FT {
+  using __bt = decltype(__T::x);
+  using __ft = typename __FT<__bt>::__ft;
----------------
jlebar wrote:
> this is c++11-only.  Which, you know what, fine by me.  But might be worth an explicit #error at least?
I've added an include guard instead. This header is included from the cuda runtime wrapper for all compilations. We don't want to break folks who use c++98, but don't need textures. If they do try to use them, they will get a static assert.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110089