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