[Clang][Sema] Generate vector vs scalar builtin overloads

[porglezomp]

Summary

The bit-shifting operator builtin support vector vs scalars, but the
operator overloads did not. That mismatch caused an assertion failure
when trying to shift a vector by an enum.

This makes C++ match the behavior of C for these conversions on
bit-shifts.

rdar://108819842
Depends on D151059

[porglezomp]

Rebasing onto modified tests

[aaron.ballman]

Thanks for the fix! The changes should come with a release note.

clang/lib/Sema/SemaOverload.cpp

9041–9044

I'm a bit confused -- the comment says this is to allow splatting the scalar to a vector, but... what is the scalar type in these loops?

[porglezomp]

clang/lib/Sema/SemaOverload.cpp

9041–9044

Right, so my goal here is to generate the two vector-operand overload candidates so that overload resolution can pick one of those and then apply the implicit scalar->vector conversion with the splat. I think this is the correct approach (it's based on how the compound assignment operators already handle this) but I need to improve my comments here.

Also, how do we like the loop here for trying both sides, as opposed to duplicating the code? This is really acting as an if left operand / else if right operand to avoid duplicating the code but I was worried writing it that the loop obscures the intent. Maybe that's also just "make the comment better" or maybe that part is clear enough.

[aaron.ballman]

clang/lib/Sema/SemaOverload.cpp

9041–9044

Right, so my goal here is to generate the two vector-operand overload candidates so that overload resolution can pick one of those and then apply the implicit scalar->vector conversion with the splat. I think this is the correct approach (it's based on how the compound assignment operators already handle this) but I need to improve my comments here.

Huh... that code looks equally as suspicious to me as well (for the same reason, it's looping over the same vector types twice). @fhahn authored that code, so perhaps he can educate me on what I'm missing.

Also, how do we like the loop here for trying both sides, as opposed to duplicating the code? This is really acting as an if left operand / else if right operand to avoid duplicating the code but I was worried writing it that the loop obscures the intent. Maybe that's also just "make the comment better" or maybe that part is clear enough.

It too me a hot minute to figure out what the code was doing, but once I picked up on this being a way to avoid code duplication, I was happy enough with it. It might be slightly more clear to refactor to use a lambda for the actual logic and then you can do if left operand/else without duplication.

[porglezomp]

Confirming your sense that those loops look a bit suspicious, I've found that this makes some implicit conversions ambiguous.
Put a test case up for that at D151532, I think I understand what those loops should be changed to instead, so I'll update when I have something passing this new test as well.

clang/lib/Sema/SemaOverload.cpp

for (unsigned Left = FirstPromotedIntegralType;
          Left < LastPromotedIntegralType; ++Left) {
       for (unsigned Right = FirstPromotedIntegralType;
            Right < LastPromotedIntegralType; ++Right) {
         QualType LandR[2] = { ArithmeticTypes[Left],
                               ArithmeticTypes[Right] };
         S.AddBuiltinCandidate(LandR, Args, CandidateSet);
       }
     }
+
+    // Extension: Add the binary bitwise operators for vector types
+    if (!CandidateTypes[0].vector_types().empty() &&
+        !CandidateTypes[1].vector_types().empty()) {
+      // If both candidates have vector types, then add them pairwise...
+      for (QualType Vec1Ty : CandidateTypes[0].vector_types()) {
+        for (QualType Vec2Ty : CandidateTypes[1].vector_types()) {
+          QualType LandR[2] = {Vec1Ty, Vec2Ty};
+          S.AddBuiltinCandidate(LandR, Args, CandidateSet);
+        }
+      }
+    } else {
+      // ...but if only one side has vector candidates, use that set of
+      // vector candidates pairwise for both sides of the operator
+      // (allowing splatting the scalar to a vector).
+      for (unsigned Candidate = 0; Candidate < 2; ++Candidate) {
+        for (QualType Vec1Ty : CandidateTypes[Candidate].vector_types()) {
+          for (QualType Vec2Ty : CandidateTypes[Candidate].vector_types()) {
+            QualType LandR[2] = {Vec1Ty, Vec2Ty};
+            S.AddBuiltinCandidate(LandR, Args, CandidateSet);
+          }
+        }
+      }
+    }
   }
 
   // C++ [over.built]p20:
   //
   //   For every pair (T, VQ), where T is an enumeration or
   //   pointer to member type and VQ is either volatile or
   //   empty, there exist candidate operator functions of the form
   //
for (unsigned Left = FirstIntegralType; Left < LastIntegralType; ++Left) {
         forAllQualifierCombinations(
             VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
               ParamTypes[0] =
                   makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
               S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
             });
       }
     }
+
+    // Extension: Add the binary operators %=, <<=, >>=, &=, ^=, |= for vector types.
+    for (QualType Vec1Ty : CandidateTypes[0].vector_types())
+      for (QualType Vec2Ty : CandidateTypes[0].vector_types()) {
+        QualType ParamTypes[2];
+        ParamTypes[1] = Vec2Ty;
+        // Add this built-in operator as a candidate (VQ is empty).
+        ParamTypes[0] = S.Context.getLValueReferenceType(Vec1Ty);
+        S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
+                              /*IsAssignmentOperator=*/false);
+
+        // Add this built-in operator as a candidate (VQ is 'volatile').
+        if (VisibleTypeConversionsQuals.hasVolatile()) {
+          ParamTypes[0] = S.Context.getVolatileType(Vec1Ty);
+          ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
+          S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
+                                /*IsAssignmentOperator=*/false);
+        }
+      }
   }
 
   // C++ [over.operator]p23:
   //
   //   There also exist candidate operator functions of the form
   //
   //        bool        operator!(bool);
   //        bool        operator&&(bool, bool);

clang/test/SemaCXX/vector.cpp

void test_enum_vector_scalar(Enum ea, v2u v2ua) {
   // expected-error@-1{{invalid operands to binary expression}}
 
   (void)(v2ua & ea); // expected-error{{cannot convert between vector values of different size}}
   (void)(ea & v2ua); // expected-error{{cannot convert between vector values of different size}}
   (void)(v2ua | ea); // expected-error{{cannot convert between vector values of different size}}
   (void)(ea | v2ua); // expected-error{{cannot convert between vector values of different size}}
   (void)(v2ua ^ ea); // expected-error{{cannot convert between vector values of different size}}
   (void)(ea ^ v2ua); // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // https://github.com/llvm/llvm-project/issues/62870
-  // (void)(v2ua << ea);
-  // (void)(ea << v2ua);
-  // (void)(v2ua >> ea);
-  // (void)(ea >> v2ua);
+  (void)(v2ua << ea);
+  (void)(ea << v2ua);
+  (void)(v2ua >> ea);
+  (void)(ea >> v2ua);
 
   v2ua += ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua -= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua *= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua /= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua %= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua &= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua |= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua ^= ea; // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // https://github.com/llvm/llvm-project/issues/62870
-  // v2ua >>= ea;
-  // v2ua <<= ea;
+  v2ua >>= ea;
+  v2ua <<= ea;
 
   ea += v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea -= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea *= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea /= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea %= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea &= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea |= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea ^= v2ua; // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // https://github.com/llvm/llvm-project/issues/62870
-  // ea >>= v2ua; // not-expected-error{{assigning to 'enum Enum' from incompatible type 'v2u'}}
-  // ea <<= v2ua; // not-expected-error{{assigning to 'enum Enum' from incompatible type 'v2u'}}
+  ea >>= v2ua; // expected-error{{assigning to 'Enum' from incompatible type 'v2u'}}
+  ea <<= v2ua; // expected-error{{assigning to 'Enum' from incompatible type 'v2u'}}
 }
 
 #if __cplusplus >= 201103L // C++11 or later
 enum class EnumClass { ENUM };
 
 void test_scoped_enum_vector(EnumClass ea, v2u v2ua) {
   // Scoped enumerations are only compatible with exactly matching types. They shouldn't integral promote.
   (void)(v2ua + ea); // expected-error{{cannot convert between vector and non-scalar values}}
   (void)(ea + v2ua); // expected-error{{cannot convert between vector and non-scalar values}}
 }
 #endif
 }