Fix bitcast to address space cast for coerced load/stores

[david-salinas]

Summary

Coerced load/stores through memory do not take into account potential address space differences when it creates its bitcasts.

[david-salinas]

remove attribute((cpu)) attribute((hc)) from test

[rjmccall]

lib/CodeGen/CGCall.cpp

1260

The main reason why llvm.memcpy is an overloaded intrinsic is so that you can copy things in arbitrary address spaces. Please just cast to the appropriate i8 pointer type in the corresponding address spaces.

[david-salinas]

As per rjmccall's comments, changed the call to CreatePointerBitCastOrAddrSpaceCast back to a call to CreateBitCast, but now we're casting to the int8* with the address space of the target.

[rjmccall]

lib/CodeGen/CGCall.cpp

1257

CGBuilder provides a convenient CreateElementBitCast function for this.

[david-salinas]

using CreateElementBitCast instead

[david-salinas]

fix missing ;

[rjmccall]

Thanks! LGTM.

lib/CodeGen/CGCall.cpp

if (SrcSize >= DstSize) {
     // to that information.
     Src = CGF.Builder.CreateBitCast(Src,
                                     Ty->getPointerTo(Src.getAddressSpace()));
     return CGF.Builder.CreateLoad(Src);
   }
 
   // Otherwise do coercion through memory. This is stupid, but simple.
   Address Tmp = CreateTempAllocaForCoercion(CGF, Ty, Src.getAlignment());
-  Address Casted = CGF.Builder.CreateBitCast(Tmp, CGF.AllocaInt8PtrTy);
-  Address SrcCasted = CGF.Builder.CreateBitCast(Src, CGF.AllocaInt8PtrTy);
+  Address Casted = CGF.Builder.CreateBitCast(Tmp, CGF.Int8Ty->getPointerTo(Tmp.getAddressSpace()));
+  Address SrcCasted = CGF.Builder.CreateBitCast(Src, CGF.Int8Ty->getPointerTo(Src.getAddressSpace()));
   CGF.Builder.CreateMemCpy(Casted, SrcCasted,
       llvm::ConstantInt::get(CGF.IntPtrTy, SrcSize),
       false);
   return CGF.Builder.CreateLoad(Tmp);
 }
 
 // Function to store a first-class aggregate into memory.  We prefer to
 // store the elements rather than the aggregate to be more friendly to
 // fast-isel.
 // FIXME: Do we need to recurse here?
 static void BuildAggStore(CodeGenFunction &CGF, llvm::Value *Val,
if (SrcSize <= DstSize) {
     // Generally SrcSize is never greater than DstSize, since this means we are
     // losing bits. However, this can happen in cases where the structure has
     // additional padding, for example due to a user specified alignment.
     //
     // FIXME: Assert that we aren't truncating non-padding bits when have access
     // to that information.
     Address Tmp = CreateTempAllocaForCoercion(CGF, SrcTy, Dst.getAlignment());
     CGF.Builder.CreateStore(Src, Tmp);
-    Address Casted = CGF.Builder.CreateBitCast(Tmp, CGF.AllocaInt8PtrTy);
-    Address DstCasted = CGF.Builder.CreateBitCast(Dst, CGF.AllocaInt8PtrTy);
+    Address Casted = CGF.Builder.CreateBitCast(Tmp,CGF.Int8Ty->getPointerTo(Tmp.getAddressSpace()) );
+    Address DstCasted = CGF.Builder.CreateBitCast(Dst, CGF.Int8Ty->getPointerTo(Dst.getAddressSpace()));
     CGF.Builder.CreateMemCpy(DstCasted, Casted,
         llvm::ConstantInt::get(CGF.IntPtrTy, DstSize),
         false);
   }
 }
 
 static Address emitAddressAtOffset(CodeGenFunction &CGF, Address addr,
                                    const ABIArgInfo &info) {

test/CodeGenCXX/address-space-cast-coerce.cpp

+// RUN: %clang_cc1 %s -triple=amdgcn-amd-amdhsa -emit-llvm -o - | FileCheck %s
+
+template<typename T, unsigned int n> struct my_vector_base;
+
+    template<typename T>
+    struct my_vector_base<T, 1> {
+        typedef T Native_vec_ __attribute__((ext_vector_type(1)));
+
+        union {
+            Native_vec_ data;
+            struct {
+                T x;
+            };
+        };
+    };
+
+    template<typename T, unsigned int rank>
+    struct my_vector_type : public my_vector_base<T, rank> {
+        using my_vector_base<T, rank>::data;
+        using typename my_vector_base<T, rank>::Native_vec_;
+
+        template< typename U>
+        __attribute__((cpu)) __attribute__((hc))
+        my_vector_type(U x) noexcept
+        {
+            for (auto i = 0u; i != rank; ++i) data[i] = x;
+        }
+        __attribute__((cpu)) __attribute__((hc))
+        my_vector_type& operator+=(const my_vector_type& x) noexcept
+        {
+            data += x.data;
+            return *this;
+        }
+    };
+
+template<typename T, unsigned int n>
+    __attribute__((cpu)) __attribute__((hc))
+    inline
+    my_vector_type<T, n> operator+(
+        const my_vector_type<T, n>& x, const my_vector_type<T, n>& y) noexcept
+    {
+        return my_vector_type<T, n>{x} += y;
+    }
+
+using char1 = my_vector_type<char, 1>;
+
+int main() {
+
+    char1 f1{1};
+    char1 f2{1};
+
+// CHECK: %[[a:[^ ]+]] = addrspacecast i16 addrspace(5)* %{{[^ ]+}} to i16*
+// CHECK: %[[a:[^ ]+]] = addrspacecast %{{[^ ]+}} addrspace(5)* %{{[^ ]+}} to %{{[^ ]+}} 
+
+    char1 f3 = f1 + f2;
+}
+
+/*
+
+Look for this:
+HECK:  %4 = addrspacecast i16 addrspace(5)* %tmp to i16*
+HECK:  %5 = addrspacecast %struct.my_vector_type* addrspace(5)* %this.addr.i to %struct.my_vector_type**
+
+FAIL<
+pass>
+38,39c38,39
+<   %5 = bitcast i16* %4 to i8 addrspace(5)*
+<   %6 = bitcast <1 x i8>* %coerce.dive2 to i8 addrspace(5)*
+---
+>   %5 = addrspacecast i16* %4 to i8 addrspace(5)*
+>   %6 = addrspacecast <1 x i8>* %coerce.dive2 to i8 addrspace(5)*
+86,87c86,87
+<   %11 = bitcast i16* %4 to i8 addrspace(5)*
+<   %12 = bitcast <1 x i8>* %coerce.dive2 to i8 addrspace(5)*
+---
+>   %11 = addrspacecast i16* %4 to i8 addrspace(5)*
+>   %12 = addrspacecast <1 x i8>* %coerce.dive2 to i8 addrspace(5)*
+
+
+*/
+