[PATCH] D75685: Add MS Mangling for OpenCL Pipe types, add mangling test.

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 3 inline comments as done.
erichkeane added inline comments.


================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:2956
+
+  mangleArtificialTagType(TTK_Struct, TemplateMangling, {"__clang"});
 }
----------------
Anastasia wrote:
> We don't seem to add namespace for other OpenCL types, although I am not against it as I find it actually cleaner.
> 
> Since the mangling deviates what is documented can you add some comments here explaining your mangling scheme?
The Microsoft mangling scheme is owned by the Microsoft Corporation, so adding something to their mangling (like 8ocl_pipe) isn't permitted.  Thus, the clang project mangles our types that aren't supported by Microsoft as a type in the __clang namespace.  

You'll note that we do it for a bunch of types above, including AddressSpaceType, VectorType, _Complex, _Float16, _Half, etc.


================
Comment at: clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl:20
+//  or write/read. Our Windows mangling does, so make sure this still works.
+void test2(read_only pipe int p) {
+// WINDOWS: define dso_local void @"?test2@@YAXU?$ocl_pipe at H$00 at __clang@@@Z"
----------------
Anastasia wrote:
> any reason this is different from the rest?
Sorry, I don't understand the question perhaps? (different in what way?).  

Do you mean why it is in a macro? I wanted an example to show that overloading works so I chose test2.  As the comment says, the OpenCL standard mangling doesn't support overloading on just pipes (since they don't take element type and read/write into account).  

In those cases, we get a code-gen emitted diagnostic that two functions have identical mangling.


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

https://reviews.llvm.org/D75685