[Sema] Adjust address space of operands in remaining builtin operators

[Anastasia]

Summary

Fix address space of reference operand in the remaining builtin compound assignment operators.

[rjmccall]

This patch LGTM.

You might consider adding tests for weird cases like class types with conversion operators to reference types. To a certain extent that sort of thing is unimplementable because the space of address spaces is infinite, so we can't fall back on adding a candidate for every possible address space; but even if we don't plan on addressing it, we can test our current behavior to check that we don't do something unexpected (like crash or give a nonsensical diagnostic).

[Anastasia]

In D59367#1434353, @rjmccall wrote:

This patch LGTM.

You might consider adding tests for weird cases like class types with conversion operators to reference types. To a certain extent that sort of thing is unimplementable because the space of address spaces is infinite, so we can't fall back on adding a candidate for every possible address space; but even if we don't plan on addressing it, we can test our current behavior to check that we don't do something unexpected (like crash or give a nonsensical diagnostic).

Thanks for the suggestion. I will look into this!

lib/Sema/SemaOverload.cpp

void addAssignmentArithmeticOverloads(bool isEqualOp) {
     if (!HasArithmeticOrEnumeralCandidateType)
       return;
 
     for (unsigned Left = 0; Left < NumArithmeticTypes; ++Left) {
       for (unsigned Right = FirstPromotedArithmeticType;
            Right < LastPromotedArithmeticType; ++Right) {
         QualType ParamTypes[2];
         ParamTypes[1] = ArithmeticTypes[Right];
-
+        auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
+            S, ArithmeticTypes[Left], Args[0]);
         // Add this built-in operator as a candidate (VQ is empty).
-        ParamTypes[0] =
-          S.Context.getLValueReferenceType(ArithmeticTypes[Left]);
+        ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
         S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
                               /*IsAssigmentOperator=*/isEqualOp);
 
         // Add this built-in operator as a candidate (VQ is 'volatile').
         if (VisibleTypeConversionsQuals.hasVolatile()) {
-          ParamTypes[0] =
-            S.Context.getVolatileType(ArithmeticTypes[Left]);
+          ParamTypes[0] = S.Context.getVolatileType(LeftBaseTy);
           ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
           S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
                                 /*IsAssigmentOperator=*/isEqualOp);
         }
       }
     }
 
     // Extension: Add the binary operators =, +=, -=, *=, /= for vector types.
void addAssignmentIntegralOverloads() {
     if (!HasArithmeticOrEnumeralCandidateType)
       return;
 
     for (unsigned Left = FirstIntegralType; Left < LastIntegralType; ++Left) {
       for (unsigned Right = FirstPromotedIntegralType;
            Right < LastPromotedIntegralType; ++Right) {
         QualType ParamTypes[2];
         ParamTypes[1] = ArithmeticTypes[Right];
-
+        auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
+            S, ArithmeticTypes[Left], Args[0]);
         // Add this built-in operator as a candidate (VQ is empty).
-        ParamTypes[0] = S.Context.getLValueReferenceType(
-            AdjustAddressSpaceForBuiltinOperandType(S, ArithmeticTypes[Left],
-                                                    Args[0]));
+        ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
         S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
         if (VisibleTypeConversionsQuals.hasVolatile()) {
           // Add this built-in operator as a candidate (VQ is 'volatile').
-          ParamTypes[0] = ArithmeticTypes[Left];
+          ParamTypes[0] = LeftBaseTy;
           ParamTypes[0] = S.Context.getVolatileType(ParamTypes[0]);
           ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
           S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
         }
       }
     }
   }
 

test/CodeGenOpenCLCXX/addrspace-operators.cl

 //RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
 
 enum E {
   a,
   b,
 };
 
 class C {
 public:
   void Assign(E e) { me = e; }
   void OrAssign(E e) { mi |= e; }
   E me;
   int mi;
 };
 
 __global E globE;
+volatile __global int globVI;
+__global int globI;
 //CHECK-LABEL: define spir_func void @_Z3barv()
 void bar() {
   C c;
   //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
   //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
   c.Assign(a);
   //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
   //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
   c.OrAssign(a);
 
   E e;
-  // CHECK: store i32 1, i32* %e
+  //CHECK: store i32 1, i32* %e
   e = b;
-  // CHECK: store i32 0, i32 addrspace(1)* @globE
+  //CHECK: store i32 0, i32 addrspace(1)* @globE
   globE = a;
-  // FIXME: Sema fails here because it thinks the types are incompatible.
-  //e = b;
-  //globE = a;
+  //CHECK: store i32 %or, i32 addrspace(1)* @globI
+  globI |= b;
+  //CHECK: store i32 %add, i32 addrspace(1)* @globI
+  globI += a;
+  //CHECK: store volatile i32 %and, i32 addrspace(1)* @globVI
+  globVI &= b;
+  //CHECK: store volatile i32 %sub, i32 addrspace(1)* @globVI
+  globVI -= a;
 }
 
 //CHECK: define linkonce_odr void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %this, i32 %e)
 //CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
 //CHECK: %me = getelementptr inbounds %class.C, %class.C addrspace(4)* %this1, i32 0, i32 0
 //CHECK: store i32 [[E]], i32 addrspace(4)* %me
 
 //CHECK define linkonce_odr void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %this, i32 %e)
 //CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
 //CHECK: %mi = getelementptr inbounds %class.C, %class.C addrspace(4)* %this1, i32 0, i32 1
 //CHECK: [[MI:%[0-9]+]] = load i32, i32 addrspace(4)* %mi
 //CHECK: %or = or i32 [[MI]], [[E]]