[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth created this revision.
richardmembarth added reviewers: Anastasia, aaron.ballman.
Herald added a subscriber: cfe-commits.

The current pretty-printer emits OpenCL-style memory spaces specifiers: 
__device, __constant, and __shared.
The correct CUDA memory space specifiers are: __device__, __constant__, and 
__shared__:
https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#variable-memory-space-specifiers


Repository:
  rC Clang

https://reviews.llvm.org/D54258

Files:
  lib/AST/TypePrinter.cpp


Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1738,17 +1738,19 @@
   case LangAS::opencl_private:
 break;
   case LangAS::opencl_constant:
-  case LangAS::cuda_constant:
 OS << "__constant";
 break;
   case LangAS::opencl_generic:
 OS << "__generic";
 break;
   case LangAS::cuda_device:
-OS << "__device";
+OS << "__device__";
+break;
+  case LangAS::cuda_constant:
+OS << "__constant__";
 break;
   case LangAS::cuda_shared:
-OS << "__shared";
+OS << "__shared__";
 break;
   default:
 OS << "__attribute__((address_space(";


Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1738,17 +1738,19 @@
   case LangAS::opencl_private:
 break;
   case LangAS::opencl_constant:
-  case LangAS::cuda_constant:
 OS << "__constant";
 break;
   case LangAS::opencl_generic:
 OS << "__generic";
 break;
   case LangAS::cuda_device:
-OS << "__device";
+OS << "__device__";
+break;
+  case LangAS::cuda_constant:
+OS << "__constant__";
 break;
   case LangAS::cuda_shared:
-OS << "__shared";
+OS << "__shared__";
 break;
   default:
 OS << "__attribute__((address_space(";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.

I think it's not so easy to provide such tests for CUDA.
CUDA memory space specifiers are implemented via attributes, e.g. `#define 
__shared__ __attribute__((shared))`.
As a result of this, they are pretty-printed via a different code path.
In my example, I call `Ctx.getAddrSpaceQualType(QT, LangAS::cuda_shared)`, 
which is then pretty-printed via the code above.
Any hints how to provide tests for this one?


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.

Same problem here: The CUDA memory space specifiers are represented via 
attributes, e.g. `CUDASharedAttr` and only converted in CodeGen to 
`LangAS::cuda_shared`.
We would need a different frontend that annotates `LangAS::cuda_shared`.


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.

There are external tools (e.g. hipacc ) that generate 
Clang AST. This AST uses `LangAS` annotations and emits incorrect memory space 
specifiers for CUDA when pretty-printed.


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.

CUDA maps `__shared__` internally also to `__attribute__((shared))`:

  #define __annotate__(a) \
  __attribute__((a))
  #define __location__(a) \
  __annotate__(a)
  ...
  #define __shared__ \
  __location__(shared)

My guess is that Clang does it just the same way and only converts to 
`LangAS::cuda_shared` for code generation in `GetGlobalVarAddressSpace`:
https://clang.llvm.org/doxygen/CodeGenModule_8cpp_source.html#l03305
In contrast, OpenCL uses keywords that are mapped directly to 
`LangAS::opencl_local` etc.


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.

> In https://reviews.llvm.org/D54258#1297191, @Anastasia wrote:
> 
> I agree with the change itself... but it's quite annoying that such things 
> can't be tested. :(

Yes, that's a pity :(

Is there anything missing so that this can be merged?


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.
Herald added a subscriber: ebevhan.
Herald added a project: clang.

Do you know when this will be merged?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54258



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.

Merging in two weeks is fine for me.

My assumption was that accepted patches are merged into upstream in a timely 
manner.
Maybe this is not how it works?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54258



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.

Thanks for clarification and merging!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54258



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