[OpenCL] Handle address space conversion while setting type alignment

[Anastasia]

Summary

Added missing addrspacecast case in alignment computation logic of pointer type emission in IR generation.

[yaxunl]

lib/CodeGen/CGExpr.cpp

957 ↗

(On Diff #115025)

Better assert that only CK_AddressSpaceConversion allows different addr spaces in target type.

[Anastasia]

lib/CodeGen/CGExpr.cpp

957 ↗

(On Diff #115025)

It seems for consistency then we would have to assert for both different ASes in non-addrspacecast case and equivalent ASes in the other case.

As condition becomes complicated then, I could as well split into either creating bitcast or addrspacecast explicitly at a cost of one simple check in the return statement but that would be in release build too.

[Anastasia]

lib/CodeGen/CGExpr.cpp

957 ↗

(On Diff #115025)

Sam, do you prefer an assert that will check both things or a runtime check that would make sure to build the right IR node here?

[yaxunl]

lib/CodeGen/CGExpr.cpp

957 ↗

(On Diff #115025)

splitting into bitcast and addrspacecast seems better.

[Anastasia]

Created BitCast or AddrSpaceCast conditionally.

[yaxunl]

LGTM. Thanks.

[bader]

test/CodeGenOpenCL/vectorLoadStore.cl

7 ↗

(On Diff #116345)

Can we remove this line?

15 ↗

(On Diff #116345)

Please, set SPIR target in clang options to make this check reliable.

[Anastasia]

Addressed comments from Alexey.

[bader]

LGTM. Thanks!

[alekseyshl]

vectorLoadStore.cl is failing on our bots: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/8187, please check it out

[Anastasia]

Committed in r314304

In D37804#882252, @alekseyshl wrote:

vectorLoadStore.cl is failing on our bots: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/8187, please check it out

I will commit a fix in a bit.

[Anastasia]

Committed in r314317.

cfe/trunk/lib/CodeGen/CGBuilder.h

public:
 
   using CGBuilderBaseTy::CreateBitCast;
   Address CreateBitCast(Address Addr, llvm::Type *Ty,
                         const llvm::Twine &Name = "") {
     return Address(CreateBitCast(Addr.getPointer(), Ty, Name),
                    Addr.getAlignment());
   }
 
+  using CGBuilderBaseTy::CreateAddrSpaceCast;
+  Address CreateAddrSpaceCast(Address Addr, llvm::Type *Ty,
+                              const llvm::Twine &Name = "") {
+    return Address(CreateAddrSpaceCast(Addr.getPointer(), Ty, Name),
+                   Addr.getAlignment());
+  }
+
   /// Cast the element type of the given address to a different type,
   /// preserving information like the alignment and address space.
   Address CreateElementBitCast(Address Addr, llvm::Type *Ty,
                                const llvm::Twine &Name = "") {
     auto PtrTy = Ty->getPointerTo(Addr.getAddressSpace());
     return CreateBitCast(Addr, PtrTy, Name);
   }
 

cfe/trunk/lib/CodeGen/CGExpr.cpp

Address CodeGenFunction::EmitPointerWithAlignment(const Expr *E,
   if (const CastExpr *CE = dyn_cast<CastExpr>(E)) {
     if (const auto *ECE = dyn_cast<ExplicitCastExpr>(CE))
       CGM.EmitExplicitCastExprType(ECE, this);
 
     switch (CE->getCastKind()) {
     // Non-converting casts (but not C's implicit conversion from void*).
     case CK_BitCast:
     case CK_NoOp:
+    case CK_AddressSpaceConversion:
       if (auto PtrTy = CE->getSubExpr()->getType()->getAs<PointerType>()) {
         if (PtrTy->getPointeeType()->isVoidType())
           break;
 
         LValueBaseInfo InnerInfo;
         Address Addr = EmitPointerWithAlignment(CE->getSubExpr(), &InnerInfo);
         if (BaseInfo) *BaseInfo = InnerInfo;
 
case CK_AddressSpaceConversion:
         if (SanOpts.has(SanitizerKind::CFIUnrelatedCast) &&
             CE->getCastKind() == CK_BitCast) {
           if (auto PT = E->getType()->getAs<PointerType>())
             EmitVTablePtrCheckForCast(PT->getPointeeType(), Addr.getPointer(),
                                       /*MayBeNull=*/true,
                                       CodeGenFunction::CFITCK_UnrelatedCast,
                                       CE->getLocStart());
         }
-
-        return Builder.CreateBitCast(Addr, ConvertType(E->getType()));
+        return CE->getCastKind() != CK_AddressSpaceConversion
+                   ? Builder.CreateBitCast(Addr, ConvertType(E->getType()))
+                   : Builder.CreateAddrSpaceCast(Addr,
+                                                 ConvertType(E->getType()));
       }
       break;
 
     // Array-to-pointer decay.
     case CK_ArrayToPointerDecay:
       return EmitArrayToPointerDecay(CE->getSubExpr(), BaseInfo);
 
     // Derived-to-base conversions.

cfe/trunk/test/CodeGenOpenCL/vectorLoadStore.cl

-// RUN: %clang_cc1 %s -emit-llvm -O0 -o - | FileCheck %s
+// RUN: %clang_cc1 -triple "spir-unknown-unknown" %s -emit-llvm -O0 -o - | FileCheck %s
 
-typedef char char3 __attribute((ext_vector_type(3)));;
+typedef char char2 __attribute((ext_vector_type(2)));
+typedef char char3 __attribute((ext_vector_type(3)));
+typedef char char8 __attribute((ext_vector_type(8)));
+typedef float float4 __attribute((ext_vector_type(4)));
 
 // Check for optimized vec3 load/store which treats vec3 as vec4.
 void foo(char3 *P, char3 *Q) {
   *P = *Q;
   // CHECK: %{{.*}} = shufflevector <4 x i8> %{{.*}}, <4 x i8> undef, <3 x i32> <i32 0, i32 1, i32 2>
 }
+
+// CHECK: define spir_func void @alignment()
+void alignment() {
+  __private char2 data_generic[100];
+  __private char8 data_private[100];
+
+  // CHECK: %{{.*}} = load <4 x float>, <4 x float> addrspace(4)* %{{.*}}, align 2
+  // CHECK: store <4 x float> %{{.*}}, <4 x float>* %{{.*}}, align 8
+  ((private float4 *)data_private)[1] = ((float4 *)data_generic)[2];
+}