bader added inline comments.

================
Comment at: lib/CodeGen/CGDecl.cpp:1272
+    if (getLangOpts().OpenCL) {
+      UA = llvm::GlobalValue::UnnamedAddr::None;
+      AS = CGM.getContext().getTargetAddressSpace(LangAS::opencl_constant);
----------------
Anastasia wrote:
> bader wrote:
> > AlexeySotkin wrote:
> > > Anastasia wrote:
> > > > Why this change?
> > > Without this change, global variables with unnamed address space are 
> > > translated to SPIR-V as variables with "Function" storage class, which is 
> > > wrong.
> > > This should fix this issue: 
> > > https://github.com/KhronosGroup/SPIRV-LLVM/issues/50
> > There is inconsistency with how Clang maps initializers to OpenCL memory 
> > model.
> > Consider example from the test case:
> > ```
> > __private int arr[] = {1, 2, 3};
> > ```
> > This code allocates a global constant for initializer {1, 2, 3} and later 
> > copies values from this global constant to the private array using memcpy 
> > intrinsic. The global constant must be allocated in constant address space, 
> > but old code do assign any address space, which is considered to be a 
> > private memory region.
> > This patch puts global constant objects to constant address space.
> Yes, this is perfectly fine. I was specifically asking about setting 
> llvm::GlobalValue::UnnamedAddr::None attribute. I can't see why this has to 
> be done or is it because we now assign concrete address space and private was 
> treated as no address space at all?
This attribute has nothing to do with address spaces.
See http://llvm.org/docs/LangRef.html#global-variables for local_unnamed_addr 
and unnamed_addr attributes description.
My understanding is that llvm::GlobalValue::UnnamedAddr::Global should be fine 
here.


https://reviews.llvm.org/D25305



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

Reply via email to