[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/test/CodeGenOpenCL/pipe_types_mangling.cl:12
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// UNMANGLED: define {{.*}}void @test1(
+}

erichkeane wrote:
> Anastasia wrote:
> > erichkeane wrote:
> > > Anastasia wrote:
> > > > Any reason to test unmangled?
> > > Because you asked to validate the OpenCL cases as well.
> > 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.
> Done, I wasn't able to remove OCLWINDOWS, since windows mangles attribute 
> 'overloadable' in C only (and thus OpenCL).
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...



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:20
+// Note: overloadable attribute makes OpenCL Linux still mangle this,
+// but we cannot overload on pipe still.
+// OCLLINUX: define void @_Z5test28ocl_pipe

Do you mean overload on pipe element type?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75685



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

[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/test/CodeGenOpenCL/pipe_types_mangling.cl:12
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// UNMANGLED: define {{.*}}void @test1(
+}

Anastasia wrote:
> erichkeane wrote:
> > Anastasia wrote:
> > > erichkeane wrote:
> > > > Anastasia wrote:
> > > > > Any reason to test unmangled?
> > > > Because you asked to validate the OpenCL cases as well.
> > > 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.
> > Done, I wasn't able to remove OCLWINDOWS, since windows mangles attribute 
> > 'overloadable' in C only (and thus OpenCL).
> 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...
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75685



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

[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 a comment.

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




Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:12
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// UNMANGLED: define {{.*}}void @test1(
+}

Anastasia wrote:
> erichkeane wrote:
> > Anastasia wrote:
> > > Any reason to test unmangled?
> > Because you asked to validate the OpenCL cases as well.
> 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.
Done, I wasn't able to remove OCLWINDOWS, since windows mangles attribute 
'overloadable' in C only (and thus OpenCL).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75685



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

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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

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




Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:12
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// UNMANGLED: define {{.*}}void @test1(
+}

erichkeane wrote:
> Anastasia wrote:
> > Any reason to test unmangled?
> Because you asked to validate the OpenCL cases as well.
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.



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:25
+
+#ifdef WIN
+// SPIR Spec specifies mangling on pipes that doesn't include the element type

erichkeane wrote:
> Anastasia wrote:
> > I am still unclear why is this special case?
> 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).
> 
> 
Oh I see. I think it would be good to explain this better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75685



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

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

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG86e0a6c60627: Add MS Mangling for OpenCL Pipe types, add 
mangling test. (authored by erichkeane).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D75685?vs=249367=252579#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75685

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenOpenCL/pipe_types_mangling.cl

Index: clang/test/CodeGenOpenCL/pipe_types_mangling.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/pipe_types_mangling.cl
@@ -0,0 +1,72 @@
+// 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
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s --check-prefixes=UNMANGLED,OCLLINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-pc -emit-llvm -O0 -cl-std=CL2.0 -o - %s -DWIN| FileCheck %s --check-prefixes=UNMANGLED,OCLWINDOWS
+
+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"
+// UNMANGLED: define {{.*}}void @test1(
+}
+
+__attribute__((overloadable))
+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"
+// Note: overloadable attribute makes OpenCL Linux still mangle this,
+// but we cannot overload on pipe still.
+// OCLLINUX: define void @_Z5test28ocl_pipe
+// OCLWINDOWS: define dso_local void @"?test2@@$$J0YAXU?$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.
+__attribute__((overloadable))
+void test2(read_only pipe int p) {
+// WINDOWS: define dso_local void @"?test2@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// OCLWINDOWS: define dso_local void @"?test2@@$$J0YAXU?$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"
+// UNMANGLED: define {{.*}}void @test3(
+}
+
+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"
+// UNMANGLED: define {{.*}}void @test4(
+}
+
+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"
+// UNMANGLED: define {{.*}}void @test5(
+}
+
+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
+// UNMANGLED: define {{.*}}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"
+// UNMANGLED: define {{.*}}void @test_reserved_read_pipe(
+}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -2940,11 +2940,17 @@
 
 void MicrosoftCXXNameMangler::mangleType(const PipeType *T, Qualifiers,
  SourceRange Range) {
-  DiagnosticsEngine  = 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(GlobalDecl GD,
___
cfe-commits mailing list

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D75685



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

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


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

https://reviews.llvm.org/D75685



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

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

Thanks!


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

https://reviews.llvm.org/D75685



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ping!


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

https://reviews.llvm.org/D75685



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked an inline comment as done.
erichkeane added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:2956
+
+  mangleArtificialTagType(TTK_Struct, TemplateMangling, {"__clang"});
 }

erichkeane wrote:
> Anastasia wrote:
> > erichkeane wrote:
> > > 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.
> > Are you saying that we have a bug for the other OpenCL type i.e. images, 
> > samplers, etc?
> 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.
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).




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

https://reviews.llvm.org/D75685



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

[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:
> erichkeane wrote:
> > 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.
> Are you saying that we have a bug for the other OpenCL type i.e. images, 
> samplers, etc?
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.



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:12
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// UNMANGLED: define {{.*}}void @test1(
+}

Anastasia wrote:
> Any reason to test unmangled?
Because you asked to validate the OpenCL cases as well.



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:25
+
+#ifdef WIN
+// SPIR Spec specifies mangling on pipes that doesn't include the element type

Anastasia wrote:
> I am still unclear why is this special case?
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).




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

https://reviews.llvm.org/D75685



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

[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"});
 }

erichkeane wrote:
> 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.
Are you saying that we have a bug for the other OpenCL type i.e. images, 
samplers, etc?



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:12
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// UNMANGLED: define {{.*}}void @test1(
+}

Any reason to test unmangled?



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:25
+
+#ifdef WIN
+// SPIR Spec specifies mangling on pipes that doesn't include the element type

I am still unclear why is this special case?


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

https://reviews.llvm.org/D75685



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

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

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

LGTM. Thanks!


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

https://reviews.llvm.org/D75685



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ping! Anyone else have feedback?


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

https://reviews.llvm.org/D75685



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 249367.
erichkeane added a comment.

Thanks @Anastasia  for the review!  Comments inline.

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


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

https://reviews.llvm.org/D75685

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenOpenCL/pipe_types_mangling.cl

Index: clang/test/CodeGenOpenCL/pipe_types_mangling.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/pipe_types_mangling.cl
@@ -0,0 +1,72 @@
+// 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
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s --check-prefixes=UNMANGLED,OCLLINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-pc -emit-llvm -O0 -cl-std=CL2.0 -o - %s -DWIN| FileCheck %s --check-prefixes=UNMANGLED,OCLWINDOWS
+
+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"
+// UNMANGLED: define {{.*}}void @test1(
+}
+
+__attribute__((overloadable))
+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"
+// Note: overloadable attribute makes OpenCL Linux still mangle this,
+// but we cannot overload on pipe still.
+// OCLLINUX: define void @_Z5test28ocl_pipe
+// OCLWINDOWS: define dso_local void @"?test2@@$$J0YAXU?$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.
+__attribute__((overloadable))
+void test2(read_only pipe int p) {
+// WINDOWS: define dso_local void @"?test2@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// OCLWINDOWS: define dso_local void @"?test2@@$$J0YAXU?$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"
+// UNMANGLED: define {{.*}}void @test3(
+}
+
+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"
+// UNMANGLED: define {{.*}}void @test4(
+}
+
+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"
+// UNMANGLED: define {{.*}}void @test5(
+}
+
+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
+// UNMANGLED: define {{.*}}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"
+// UNMANGLED: define {{.*}}void @test_reserved_read_pipe(
+}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -2943,11 +2943,17 @@
 
 void MicrosoftCXXNameMangler::mangleType(const PipeType *T, Qualifiers,
  SourceRange Range) {
-  DiagnosticsEngine  = 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,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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@H$00@__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



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

[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@H$00@__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



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

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


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

https://reviews.llvm.org/D75685



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: bader, Anastasia, pxli168.
Herald added a subscriber: yaxunl.

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)
"void __cdecl test2(struct __clang::ocl_pipe)
"void __cdecl test2(struct __clang::ocl_pipe)
"void __cdecl test3(struct __clang::ocl_pipe)
"void __cdecl test4(struct __clang::ocl_pipe,1>)
"void __cdecl test5(struct __clang::ocl_pipe,1>)
"void __cdecl test_reserved_read_pipe(struct __clang::_ASCLglobal * __ptr64,struct __clang::ocl_pipe)


https://reviews.llvm.org/D75685

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl


Index: clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl
@@ -0,0 +1,57 @@
+// 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"
+}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -2943,11 +2943,17 @@
 
 void MicrosoftCXXNameMangler::mangleType(const PipeType *T, Qualifiers,
  SourceRange Range) {
-  DiagnosticsEngine  = 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,


Index: clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl
@@ -0,0 +1,57 @@
+// 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