Sema: Fix explicit address space cast involving void pointers

[yaxunl]

Summary

Explicit cast of a void pointer to a pointer type in different address space is
incorrectly classified as bitcast, which causes invalid bitcast in codegen.

The patch fixes that by checking the address space of the source and destination
type and set the correct cast kind.

[rjmccall]

One minor request; feel free to just do that when you commit.

lib/Sema/SemaCast.cpp

1050 ↗

(On Diff #158043)

I know the code was like this before, but please rewrite this to just use getAs<PointerType>() instead of doing the separate isPointerType() check.

[yaxunl]

lib/Sema/SemaCast.cpp

1050 ↗

(On Diff #158043)

IsAddressSpaceConversion is also called in line 2218 of the same file, where SrcType or DestType may not be pointer type.

[rjmccall]

lib/Sema/SemaCast.cpp

1050 ↗

(On Diff #158043)

getAs is a query; it returns null if the type isn't of the target type.

[yaxunl]

lib/Sema/SemaCast.cpp

1050 ↗

(On Diff #158043)

How about

auto *SrcPtrType = SrcType->getAs<PointerType>();
if (!SrcPtrType )
  return false;
auto *DestPtrType = DestType->getAs<PointerType>();
if (!DestPtrType)
  return false;
return SrcPtrType->getPointeeType().getAddressSpace() !=
             DestPtrType->getPointeeType().getAddressSpace();

[rjmccall]

lib/Sema/SemaCast.cpp

1050 ↗

(On Diff #158043)

That looks right.

cfe/trunk/lib/Sema/SemaCast.cpp

if (Kind == CK_BitCast)
       checkCastAlign();
     if (Self.getLangOpts().allowsNonTrivialObjCLifetimeQualifiers())
       checkObjCConversion(Sema::CCK_OtherCast);
   } else {
     SrcExpr = ExprError();
   }
 }
 
+static bool IsAddressSpaceConversion(QualType SrcType, QualType DestType) {
+  auto *SrcPtrType = SrcType->getAs<PointerType>();
+  if (!SrcPtrType)
+    return false;
+  auto *DestPtrType = DestType->getAs<PointerType>();
+  if (!DestPtrType)
+    return false;
+  return SrcPtrType->getPointeeType().getAddressSpace() !=
+         DestPtrType->getPointeeType().getAddressSpace();
+}
+
 /// TryStaticCast - Check if a static cast can be performed, and do so if
 /// possible. If @p CStyle, ignore access restrictions on hierarchy casting
 /// and casting away constness.
 static TryCastResult TryStaticCast(Sema &Self, ExprResult &SrcExpr,
                                    QualType DestType,
                                    Sema::CheckedConversionKind CCK,
                                    SourceRange OpRange, unsigned &msg,
                                    CastKind &Kind, CXXCastPath &BasePath,
if (SrcPointee->isVoidType()) {
             SrcPointeeQuals.removeObjCGCAttr();
             SrcPointeeQuals.removeObjCLifetime();
             if (DestPointeeQuals != SrcPointeeQuals &&
                 !DestPointeeQuals.compatiblyIncludes(SrcPointeeQuals)) {
               msg = diag::err_bad_cxx_cast_qualifiers_away;
               return TC_Failed;
             }
           }
-          Kind = CK_BitCast;
+          Kind = IsAddressSpaceConversion(SrcType, DestType)
+                     ? CK_AddressSpaceConversion
+                     : CK_BitCast;
           return TC_Success;
         }
 
         // Microsoft permits static_cast from 'pointer-to-void' to
         // 'pointer-to-function'.
         if (!CStyle && Self.getLangOpts().MSVCCompat &&
             DestPointee->isFunctionType()) {
           Self.Diag(OpRange.getBegin(), diag::ext_ms_cast_fn_obj) << OpRange;
static bool fixOverloadedReinterpretCastExpr(Sema &Self, QualType DestType,
   // preserves Result.
   Result = E;
   if (!Self.resolveAndFixAddressOfOnlyViableOverloadCandidate(
           Result, /*DoFunctionPointerConversion=*/true))
     return false;
   return Result.isUsable();
 }
 
-static bool IsAddressSpaceConversion(QualType SrcType, QualType DestType) {
-  return SrcType->isPointerType() && DestType->isPointerType() &&
-         SrcType->getAs<PointerType>()->getPointeeType().getAddressSpace() !=
-             DestType->getAs<PointerType>()->getPointeeType().getAddressSpace();
-}
-
 static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
                                         QualType DestType, bool CStyle,
                                         SourceRange OpRange,
                                         unsigned &msg,
                                         CastKind &Kind) {
   bool IsLValueCast = false;
 
   DestType = Self.Context.getCanonicalType(DestType);

cfe/trunk/test/CodeGenCXX/address-space-cast.cpp

 // RUN: %clang_cc1 %s -triple=amdgcn-amd-amdhsa -emit-llvm -o - | FileCheck %s
 
 #define __private__ __attribute__((address_space(5)))
 
 void func_pchar(__private__ char *x);
+void func_pvoid(__private__ void *x);
+void func_pint(__private__ int *x);
 
-void test_cast(char *gen_ptr) {
+void test_cast(char *gen_char_ptr, void *gen_void_ptr, int *gen_int_ptr) {
   // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
   // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
-  __private__ char *priv_ptr = (__private__ char *)gen_ptr;
+  __private__ char *priv_char_ptr = (__private__ char *)gen_char_ptr;
 
   // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  priv_char_ptr = (__private__ char *)gen_void_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  priv_char_ptr = (__private__ char *)gen_int_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  __private__ void *priv_void_ptr = (__private__ void *)gen_char_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  priv_void_ptr = (__private__ void *)gen_void_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  priv_void_ptr = (__private__ void *)gen_int_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i32 addrspace(5)*
+  // CHECK-NEXT: store i32 addrspace(5)* %[[cast]]
+  __private__ int *priv_int_ptr = (__private__ int *)gen_void_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pcharPU3AS5c(i8 addrspace(5)* %[[cast]])
+  func_pchar((__private__ char *)gen_char_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pcharPU3AS5c(i8 addrspace(5)* %[[cast]])
+  func_pchar((__private__ char *)gen_void_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(5)*
   // CHECK-NEXT: call void @_Z10func_pcharPU3AS5c(i8 addrspace(5)* %[[cast]])
-  func_pchar((__private__ char *)gen_ptr);
+  func_pchar((__private__ char *)gen_int_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pvoidPU3AS5v(i8 addrspace(5)* %[[cast]])
+  func_pvoid((__private__ void *)gen_char_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pvoidPU3AS5v(i8 addrspace(5)* %[[cast]])
+  func_pvoid((__private__ void *)gen_void_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pvoidPU3AS5v(i8 addrspace(5)* %[[cast]])
+  func_pvoid((__private__ void *)gen_int_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i32 addrspace(5)*
+  // CHECK-NEXT: call void @_Z9func_pintPU3AS5i(i32 addrspace(5)* %[[cast]])
+  func_pint((__private__ int *)gen_void_ptr);
 }