arichardson added a comment. In D138284#3936846 <https://reviews.llvm.org/D138284#3936846>, @erichkeane wrote:
> In D138284#3936830 <https://reviews.llvm.org/D138284#3936830>, @aaron.ballman > wrote: > >> Adding @bader as SYCL code owner. The changes look reasonable to me, but >> codegen is not my area of expertise. > > Yeah, same to me. I wrote this originally, but I think the address-space > stuff was added in the downstream by someone else (though I can't find proof > of it now!). (Unfortunately) that was me. Not the best design, but it catches lots of subtle run-time errors at compile time. Has been extremely useful for the downstream CHERI fork. ================ Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635 + Context.getTargetInfo().getConstantAddressSpace().value_or( + LangAS::Default)); llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr( ---------------- bader wrote: > > This changes the code generation for spir64 to place the globals in > > addrspace(4). I believe is correct, but it would be good for someone who is > > familiar with the target to confirm. > > Globals must reside in `sycl_global` namespace, which is `addrspace(1)` for > spir* targets. > `addrspace(4)` represents "generic" address space, which is a placeholder for > a specific address space. If we leave it `addrspace(4)` for global > definition, the compiler won't be able to infer genuine address space. Okay that's interesting - I guess it means we should not be using `getConstantAddressSpace()` here? Or getConstantAddressSpace() should actually return a value that maps to `addrspace(1)`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138284/new/ https://reviews.llvm.org/D138284 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
