arsenm added inline comments.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3500
+        if (auto *PtrTy = dyn_cast<llvm::PointerType>(PTy)) {
+          if (PtrTy->getAddressSpace() !=
+              ArgValue->getType()->getPointerAddressSpace()) {
----------------
Anastasia wrote:
> arsenm wrote:
> > Anastasia wrote:
> > > Would this be correct for OpenCL? Should we use  
> > > `isAddressSpaceSupersetOf` helper instead? Would it also sort the issue 
> > > with constant AS (at least for OpenCL)? 
> > The issue I mentioned for the other builtin is that it modifies the memory, 
> > and doesn't have to do with the casting.
> > 
> > At this point the AddrSpaceCast has to be emitted. The checking if the cast 
> > is legal I guess would be in the SemaExpr part. I know at one point I was 
> > trying to use isAddressSpaceSupersetOf in rewriteBuiltinFunctionDecl, but 
> > there was some problem with that. I think it didn't make sense with the 
> > magic where the builtin without an address space is supposed to accept any 
> > address space or something along those lines.
> Yes, I think Sema has to check it before indeed. I am not sure it works right 
> with OpenCL rules though for the Builtin functions.  Would it make sense to 
> add a negative test for this then? 
I'm not sure what this test would look like. Do you mean a test that 
erroneously is accepted now?


https://reviews.llvm.org/D47154



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

Reply via email to