[OpenCL] Setting constant address space for array initializers

[AlexeySotkin]

Summary

Setting constant address space for global constants used for memcpy-initialization of arrays.

[yaxunl]

Please add a test. Thanks.

[AlexeySotkin]

Adding test checking address space of array initializer

[yaxunl]

The test looks OK. However, the patch only contains the test now.

[AlexeySotkin]

Squashing commits

[AlexeySotkin]

Now there should be both :)

[yaxunl]

LGTM. Thanks!

[bader]

LGTM

[Anastasia]

lib/CodeGen/CGDecl.cpp

1272

Why this change?

[AlexeySotkin]

lib/CodeGen/CGDecl.cpp

1272

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

[bader]

lib/CodeGen/CGDecl.cpp

1272

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.

[Anastasia]

lib/CodeGen/CGDecl.cpp

1272

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?

[bader]

lib/CodeGen/CGDecl.cpp

1272

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.

[AlexeySotkin]

lib/CodeGen/CGDecl.cpp

1272

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);

[Anastasia]

lib/CodeGen/CGDecl.cpp

1272

Yes, I also think that leaving global should be fine here. So could we just undo the change to line 1274?

[AlexeySotkin]

Setting UnnamedAddr to Global

[AlexeySotkin]

lib/CodeGen/CGDecl.cpp

1272

Changed to Global

[Anastasia]

LGTM! Thanks!

[bader]

Committed at 285557 with updated tests.

lib/CodeGen/CGDecl.cpp

if (!constant->isNullValue() && !isa<llvm::UndefValue>(constant)) {
       Loc = Builder.CreateBitCast(Loc, constant->getType()->getPointerTo());
       emitStoresForInitAfterMemset(constant, Loc.getPointer(),
                                    isVolatile, Builder);
     }
   } else {
     // Otherwise, create a temporary global with the initializer then
     // memcpy from the global to the alloca.
     std::string Name = getStaticDeclName(CGM, D);
+    unsigned AS = 0;
+    llvm::GlobalValue::UnnamedAddr UA = llvm::GlobalValue::UnnamedAddr::Global;
+    if (getLangOpts().OpenCL) {
+      UA = llvm::GlobalValue::UnnamedAddr::None;
+      AS = CGM.getContext().getTargetAddressSpace(LangAS::opencl_constant);
+      BP = llvm::PointerType::getInt8PtrTy(getLLVMContext(), AS);
+    }
     llvm::GlobalVariable *GV =
       new llvm::GlobalVariable(CGM.getModule(), constant->getType(), true,
                                llvm::GlobalValue::PrivateLinkage,
-                               constant, Name);
+                               constant, Name, nullptr,
+                               llvm::GlobalValue::NotThreadLocal, AS);
     GV->setAlignment(Loc.getAlignment().getQuantity());
-    GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+    GV->setUnnamedAddr(UA);
 
     Address SrcPtr = Address(GV, Loc.getAlignment());
     if (SrcPtr.getType() != BP)
       SrcPtr = Builder.CreateBitCast(SrcPtr, BP);
 
     Builder.CreateMemCpy(Loc, SrcPtr, SizeVal, isVolatile);
   }
 }