[OpenCL] Fix address space for base method call (PR43145)

[svenvh]

Summary

Clang was creating an UncheckedDerivedToBase ImplicitCastExpr that was
also casting between address spaces. Insert an ImplicitCastExpr node
for doing the address space conversion.

[svenvh]

clang/lib/CodeGen/CGCall.cpp

4091 ↗

(On Diff #227733)

@Anastasia pointed out that the AST might not be correct to begin with, which could cause us to fail here.

| `-MemberExpr 0x5555558af2c0 <col:3, col:5> '<bound member function type>' .getRef 0x5555558a4008
|   `-ImplicitCastExpr 0x5555558af310 <col:3> '__generic B2' lvalue <UncheckedDerivedToBase (B2)>
|     `-DeclRefExpr 0x5555558af2a0 <col:3> 'Derived' lvalue Var 0x5555558ab290 'd' 'Derived'

The question is whether it's fine for the UncheckedDerivedToBase ImplicitCastExpr to cast from private to __generic; or do we need another AddrSpaceConversion ImplicitCastExpr node perhaps?

[Anastasia]

clang/lib/CodeGen/CGCall.cpp

4091 ↗

(On Diff #227733)

Yes, I am not sure AST is correct wrt addr spaces.

Also we should at least use performAddrSpaceCast that isn't easy here because the source addr spaces are not available at this point.

But overall I would like @rjmccall to have a look.

[rjmccall]

clang/lib/CodeGen/CGCall.cpp

4091 ↗

(On Diff #227733)

I agree that the AST should include an address-space conversion, not just a derived-to-base conversion.

[svenvh]

Rework fix to insert an addrspace conversion node into the AST instead of catching the addrspace cast in CGCall.

[rjmccall]

clang/lib/Sema/SemaExpr.cpp

2721

Both the source and dest types here are off by a level if we're working with a pointer; you need to do:

auto FromRecordTypeWithoutAS = Context.removeAddrSpaceQualType(FromRecordType);
auto FromTypeWithDestAS = Context.getAddrSpaceQualType(FromRecordTypeWithoutAS, DestAS);
if (PointerConversions)
 FromTypeWithDestAS = Context.getPointerType(FromTypeWithDestAS);

[svenvh]

Incorporate suggestions from @rjmccall and add a test for the pointer case.

[rjmccall]

LGTM.

clang/lib/Sema/SemaExpr.cpp

if (isa<FieldDecl>(Member)) {
 
     if (FromType->getAs<PointerType>()) {
       FromRecordType = FromType->getPointeeType();
       PointerConversions = true;
     } else {
       FromRecordType = FromType;
       DestType = DestRecordType;
     }
+
+    LangAS FromAS = FromRecordType.getAddressSpace();
+    LangAS DestAS = DestRecordType.getAddressSpace();
+    if (FromAS != DestAS) {
+      auto FromTyWithoutAS = Context.removeAddrSpaceQualType(FromType);
+      auto DestASTy = Context.getAddrSpaceQualType(FromTyWithoutAS, DestAS);
+      From = ImpCastExprToType(From, DestASTy, CK_AddressSpaceConversion,
+                               From->getValueKind()).get();
+    }
   } else {
     // No conversion necessary.
     return From;
   }
 
   if (DestType->isDependentType() || FromType->isDependentType())
     return From;
 

clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl

 //CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*
 
 
 // Calling base method through multiple inheritance.
 
 class B2 {
   public:
     void baseMethod() const {  }
+    int& getRef() { return bb; }
     int bb;
 };
 
 class Derived : public B, public B2 {
   public:
     void work() const { baseMethod(); }
     // CHECK-LABEL: work
     // CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
 };
 
 void pr43145(const Derived *argDerived) {
   argDerived->work();
 }
 
 // Casting from base to derived.
 
 void pr43145_2(B *argB) {
   Derived *x = (Derived*)argB;
-}
-
   // CHECK-LABEL: @_Z9pr43145_2
   // CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
+}
+
+// Assigning to reference returned by base class method through derived class.
+
+void pr43145_3(int n) {
+  Derived d;
+  d.getRef() = n;
+  // CHECK-LABEL: @_Z9pr43145_3
+  // CHECK: addrspacecast %class.Derived* %d to %class.Derived addrspace(4)*
+  // CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
+  // CHECK: call {{.*}} @_ZNU3AS42B26getRefEv
+}