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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:2956
+
+  mangleArtificialTagType(TTK_Struct, TemplateMangling, {"__clang"});
 }
----------------
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?


================
Comment at: clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl:2
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -O0 -cl-std=clc++ -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-pc -emit-llvm -O0 -cl-std=clc++ -o - %s -DWIN| FileCheck %s --check-prefixes=WINDOWS
+
----------------
Does this work for OpenCL C (although you would need to add overloading attribute)? If so maybe worth adding a RUN line too. 

If it works for OpenCL C I would move this into test/CodeGenOpenCL. In this folder we only keep what is C++ specific. Although overloading is technically C++ but we use it in C too.


================
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"
----------------
any reason this is different from the rest?


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

https://reviews.llvm.org/D75685