Anastasia added inline comments.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:3500
+ if (auto *PtrTy = dyn_cast<llvm::PointerType>(PTy)) {
+ if (PtrTy->getAddressSpace() !=
+ ArgValue->getType()->getPointerAddressSpace()) {
----------------
arsenm wrote:
> 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?
Ok, so at this point you are trying to change generation of `bitcast` to
`addrspacecast` which makes sense to me. Do we still need a `bitcast` though?
I think `addrspacecast` can be used to convert type and address space too:
The ‘addrspacecast‘ instruction converts ptrval from pty in address space n
to type pty2 in address space m.
It would be nice to add proper Sema checking for `Builtins` for address space
of pointers in OpenCL mode, but this might be more work.
================
Comment at: test/CodeGenOpenCL/numbered-address-space.cl:36
+#if 0
+// XXX: Should this compile?
+void
test_generic_as_to_builtin_parameter_explicit_cast_numeric(__attribute__((address_space(3)))
int *local_ptr, float src) {
----------------
arsenm wrote:
> Anastasia wrote:
> > `__attribute__((address_space(N)))` is not an OpenCL feature and I think
> > it's not specified in C either? But I think generally non matching address
> > spaces don't compile in Clang. So it might be useful to disallow this?
> I'm pretty sure it's a C extension. The way things seem to work now is
> address spaces are accepted anywhere and everywhere.
Yes, the line below should give an error for OpenCL?
generic int* generic_ptr = local_ptr;
https://reviews.llvm.org/D47154
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
