Add MS Mangling for OpenCL Pipe types, add mangling test.

[erichkeane]

Summary

SPIRV2.0 Spec only specifies Linux mangling, however our downstream has
use for a Windows mangling for these types.

Unfortunately, the SPIRV
spec specifies a single mangling for all pipe types, despite clang
allowing overloading on these types. Because of this, this patch
chooses to mangle the read/writability and element type for the windows
mangling.

The windows manglings in the test all demangle according to demangler:
"void cdecl test1(struct clang::ocl_pipe<int,1>)
"void cdecl test2(struct clang::ocl_pipe<float,0>)
"void cdecl test2(struct clang::ocl_pipe<int,1>)
"void cdecl test3(struct clang::ocl_pipe<int const,1>)
"void cdecl test4(struct clang::ocl_pipe<union
clang::vector<unsigned char,3>,1>)
"void cdecl test5(struct clang::ocl_pipe<union
clang::vector<int,4>,1>)
"void cdecl test_reserved_read_pipe(struct clang::_ASCLglobal<struc
Person > * ptr64,struct clang::ocl_pipe<struct Person,1>)

[erichkeane]

Ping! @Anastasia I was hoping for your input on this, since it is OpenCLC++.

[Anastasia]

clang/lib/AST/MicrosoftMangle.cpp

2956

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?

clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl

2

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.

20

any reason this is different from the rest?

[erichkeane]

clang/lib/AST/MicrosoftMangle.cpp

2956

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.

clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl

20

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.

[erichkeane]

Thanks @Anastasia for the review! Comments inline.

Moved the test as requested, and added OpenCL tests for both OSes.

[erichkeane]

Ping! Anyone else have feedback?

[bader]

LGTM. Thanks!

[Anastasia]

clang/lib/AST/MicrosoftMangle.cpp

2956

Are you saying that we have a bug for the other OpenCL type i.e. images, samplers, etc?

clang/test/CodeGenOpenCL/pipe_types_mangling.cl

12 ↗

(On Diff #249367)

Any reason to test unmangled?

25 ↗

(On Diff #249367)

I am still unclear why is this special case?

[erichkeane]

clang/lib/AST/MicrosoftMangle.cpp

2956

I don't know those well enough to answer, but at least in this case 'Pipe' isn't a Type. It is a Type Class. Because of this, having a single mangling for it is incorrect. It appears images likely is going to have this problem as well.

clang/test/CodeGenOpenCL/pipe_types_mangling.cl

12 ↗

(On Diff #249367)

Because you asked to validate the OpenCL cases as well.

25 ↗

(On Diff #249367)

It isn't possible to overload on these types in Linux mode, because the OpenCL spec on ItaniumABI has a specific mangling that doesn't take element type and read/write into effect. The probelm is that on Linux BOTH versions of 'test2' end up with the mangling '@_Z5test28ocl_pipe' (and we get an error in codegen).

[erichkeane]

clang/lib/AST/MicrosoftMangle.cpp

2956

I've looked closer, and I don't believe this is a problem for other openCL types (other than they are not reserved names). In ~2090 in this file those are implemented as a struct named ocl_*, which while not reserved in C++ will demangle. Image types are mangled differently, also as a struct type, so i think those are OK.

These pipe types aren't mangled separately in Linux (hence why overloading doesn't work with them in Linux).

[erichkeane]

Ping!

[erichkeane]

Ping! I would expect this to be non-controversial, but if there is a different reviewer anyone can suggest, I'd appreciate it!

Thanks!

[erichkeane]

Does anyone have any comments for me? This is blocking using pipes on windows targets.

[aaron.ballman]

LGTM!

[Anastasia]

Sorry for the delayed comments. It would be good to address those in a separate commit if possible.

clang/test/CodeGenOpenCL/pipe_types_mangling.cl

12 ↗

(On Diff #249367)

Ok, I guess we should add overloadable attribute otherwise it doesn't go through mangling? Then I guess you only need a check for Linux or Windows cases.

25 ↗

(On Diff #249367)

Oh I see. I think it would be good to explain this better.

[erichkeane]

Both done here: fe5c719eaf572e23b700e75832ec37a3761b337b . Thanks for the review @Anastasia

clang/test/CodeGenOpenCL/pipe_types_mangling.cl

12 ↗

(On Diff #249367)

Done, I wasn't able to remove OCLWINDOWS, since windows mangles attribute 'overloadable' in C only (and thus OpenCL).

[Anastasia]

clang/test/CodeGenOpenCL/pipe_types_mangling.cl

20 ↗

(On Diff #252579)

Do you mean overload on pipe element type?

12 ↗

(On Diff #249367)

Sorry what I meant is we should probably add overloadable attr to all functions so that we can check mangling in OpenCL C too. Then we won't need to check UNMANGLED.

I am very confused of why right now you only do it in some cases but not all...

[Anastasia]

clang/test/CodeGenOpenCL/pipe_types_mangling.cl

12 ↗

(On Diff #249367)

I just noticed that my earlier comment has been addressed in https://reviews.llvm.org/rGfe5c719eaf57 but it didn't show on this review. So we are fine now! Thanks for looking at this.

clang/lib/AST/MicrosoftMangle.cpp

void MicrosoftCXXNameMangler::mangleType(const AtomicType *T, Qualifiers,
   Extra.mangleSourceName("_Atomic");
   Extra.mangleType(ValueType, Range, QMM_Escape);
 
   mangleArtificialTagType(TTK_Struct, TemplateMangling, {"__clang"});
 }
 
 void MicrosoftCXXNameMangler::mangleType(const PipeType *T, Qualifiers,
                                          SourceRange Range) {
-  DiagnosticsEngine &Diags = Context.getDiags();
-  unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
-    "cannot mangle this OpenCL pipe type yet");
-  Diags.Report(Range.getBegin(), DiagID)
-    << Range;
+  QualType ElementType = T->getElementType();
+
+  llvm::SmallString<64> TemplateMangling;
+  llvm::raw_svector_ostream Stream(TemplateMangling);
+  MicrosoftCXXNameMangler Extra(Context, Stream);
+  Stream << "?$";
+  Extra.mangleSourceName("ocl_pipe");
+  Extra.mangleType(ElementType, Range, QMM_Escape);
+  Extra.mangleIntegerLiteral(llvm::APSInt::get(T->isReadOnly()), true);
+
+  mangleArtificialTagType(TTK_Struct, TemplateMangling, {"__clang"});
 }
 
 void MicrosoftMangleContextImpl::mangleCXXName(const NamedDecl *D,
                                                raw_ostream &Out) {
   assert((isa<FunctionDecl>(D) || isa<VarDecl>(D)) &&
          "Invalid mangleName() call, argument is not a variable or function!");
   assert(!isa<CXXConstructorDecl>(D) && !isa<CXXDestructorDecl>(D) &&
          "Invalid mangleName() call on 'structor decl!");

clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl

+// 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
+
+typedef unsigned char __attribute__((ext_vector_type(3))) uchar3;
+typedef int __attribute__((ext_vector_type(4))) int4;
+
+void test1(read_only pipe int p) {
+// LINUX: define void @_Z5test18ocl_pipe
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+}
+
+void test2(write_only pipe float p) {
+// LINUX: define void @_Z5test28ocl_pipe
+// WINDOWS: define dso_local void @"?test2@@YAXU?$ocl_pipe@M$0A@@__clang@@@Z"
+}
+
+#ifdef WIN
+// SPIR Spec specifies mangling on pipes that doesn't include the element type
+//  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@H$00@__clang@@@Z"
+}
+#endif
+
+
+void test3(read_only pipe const int p) {
+// LINUX: define void @_Z5test38ocl_pipe
+// WINDOWS: define dso_local void @"?test3@@YAXU?$ocl_pipe@$$CBH$00@__clang@@@Z"
+}
+
+void test4(read_only pipe uchar3 p) {
+// LINUX: define void @_Z5test48ocl_pipe
+// WINDOWS: define dso_local void @"?test4@@YAXU?$ocl_pipe@T?$__vector@E$02@__clang@@$00@__clang@@@Z"
+}
+
+void test5(read_only pipe int4 p) {
+// LINUX: define void @_Z5test58ocl_pipe
+// WINDOWS: define dso_local void @"?test5@@YAXU?$ocl_pipe@T?$__vector@H$03@__clang@@$00@__clang@@@Z"
+}
+
+typedef read_only pipe int MyPipe;
+kernel void test6(MyPipe p) {
+// LINUX: define spir_kernel void @test6
+// WINDOWS: define dso_local spir_kernel void @test6
+}
+
+struct Person {
+  const char *Name;
+  bool isFemale;
+  int ID;
+};
+
+void test_reserved_read_pipe(global struct Person *SDst,
+                             read_only pipe struct Person SPipe) {
+// LINUX: define void @_Z23test_reserved_read_pipePU8CLglobal6Person8ocl_pipe
+// WINDOWS: define dso_local void @"?test_reserved_read_pipe@@YAXPEAU?$_ASCLglobal@$$CAUPerson@@@__clang@@U?$ocl_pipe@UPerson@@$00@2@@Z"
+}