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

Reply via email to