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

Reply via email to