AlexeySotkin 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:
> AlexeySotkin wrote:
> > bader wrote:
> > > 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.
> > llvm::GlobalValue::UnnamedAddr::None is default value of UnnamedAddr for
> > GlobalValue. This line can be removed, but extra "if" statement must be
> > added before GV->setUnnamedAddr(UA);
> Yes, I also think that leaving global should be fine here. So could we just
> undo the change to line 1274?
Changed to Global
https://reviews.llvm.org/D25305
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
