Following the review of sethp, I have made the following changes:

-- Added diagnostic for the undefined shuffle of -1
-- Validated support for _BitInt
-- A bunch of other minnor tweaks here and there

Author: Destroyerrrocket

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10330,6 +10330,8 @@ def err_shufflevector_nonconstant_argument : Error<
 def err_shufflevector_argument_too_large : Error<
   "index for __builtin_shufflevector must be less than the total number "
   "of vector elements">;
+def err_shufflevector_minus_one_is_undefined_behavior_constexpr : Error<
+  "index for __builtin_shufflevector must be within the bounds of the input vectors in a constexpr context. An index of -1 at position %0 was found. -1 is only allowed at runtime.">;
 
 def err_convertvector_non_vector : Error<
   "first argument to __builtin_convertvector must be a vector">;

clang/lib/AST/ExprConstant.cpp

@@ -2706,8 +2706,11 @@ static bool checkFloatingPointResult(EvalInfo &Info, const Expr *E,
 static bool HandleFloatToFloatCast(EvalInfo &Info, const Expr *E,
                                    QualType SrcType, QualType DestType,
                                    APFloat &Result) {
-  assert(isa<CastExpr>(E) || isa<CompoundAssignOperator>(E) ||
-         isa<ConvertVectorExpr>(E));
+  assert((isa<CastExpr>(E) || isa<CompoundAssignOperator>(E) ||
+          isa<ConvertVectorExpr>(E)) &&
+         "HandleFloatToFloatCast has been checked with only CastExpr, "
+         "CompoundAssignOperator and ConvertVectorExpr. Please either validate "
+         "the new expression or address the root cause of this usage.");
   llvm::RoundingMode RM = getActiveRoundingMode(Info, E);
   APFloat::opStatus St;
   APFloat Value = Result;
@@ -10714,7 +10717,7 @@ namespace {
     bool VisitShuffleVectorExpr(const ShuffleVectorExpr *E);
 
     // FIXME: Missing: conditional operator (for GNU
-    //                 conditional select), shufflevector, ExtVectorElementExpr
+    //                 conditional select), ExtVectorElementExpr
   };
 } // end anonymous namespace
 
@@ -10965,27 +10968,10 @@ bool VectorExprEvaluator::VisitUnaryOperator(const UnaryOperator *E) {
   return Success(APValue(ResultElements.data(), ResultElements.size()), E);
 }
 
-static bool EvaluateVectorOrLValue(APValue &Result, EvalInfo &Info,
-                                   const Expr *E, const QualType &Type) {
-  if (!Evaluate(Result, Info, E))
-    return false;
-
-  if (Result.isLValue()) {
-    // Source of the data is an lvalue; Manually handle the lvalue as if
-    // it was an rvalue to get the current APValue.
-    LValue LValueFound;
-    LValueFound.setFrom(Info.Ctx, Result);
-    if (!handleLValueToRValueConversion(Info, E, Type, LValueFound, Result))
-      return false;
-  }
-
-  return Result.isVector();
-}
-
-static bool handleVectorConversion(EvalInfo &Info, const FPOptions FPO,
-                                   const Expr *E, QualType SourceTy,
-                                   QualType DestTy, APValue const &Original,
-                                   APValue &Result) {
+static bool handleVectorElementCast(EvalInfo &Info, const FPOptions FPO,
+                                    const Expr *E, QualType SourceTy,
+                                    QualType DestTy, APValue const &Original,
+                                    APValue &Result) {
   if (SourceTy->isIntegerType()) {
     if (DestTy->isRealFloatingType()) {
       Result = APValue(APFloat(0.0));
@@ -11015,7 +11001,7 @@ static bool handleVectorConversion(EvalInfo &Info, const FPOptions FPO,
 bool VectorExprEvaluator::VisitConvertVectorExpr(const ConvertVectorExpr *E) {
   APValue Source;
   QualType SourceVecType = E->getSrcExpr()->getType();
-  if (!EvaluateVectorOrLValue(Source, Info, E->getSrcExpr(), SourceVecType))
+  if (!EvaluateAsRValue(Info, E->getSrcExpr(), Source))
     return false;
 
   QualType DestTy = E->getType()->castAs<VectorType>()->getElementType();
@@ -11028,8 +11014,8 @@ bool VectorExprEvaluator::VisitConvertVectorExpr(const ConvertVectorExpr *E) {
   ResultElements.reserve(SourceLen);
   for (unsigned EltNum = 0; EltNum < SourceLen; ++EltNum) {
     APValue Elt;
-    if (!handleVectorConversion(Info, FPO, E, SourceTy, DestTy,
-                                Source.getVectorElt(EltNum), Elt))
+    if (!handleVectorElementCast(Info, FPO, E, SourceTy, DestTy,
+                                 Source.getVectorElt(EltNum), Elt))
       return false;
     ResultElements.push_back(std::move(Elt));
   }
@@ -11041,21 +11027,24 @@ static bool handleVectorShuffle(EvalInfo &Info, const ShuffleVectorExpr *E,
                                 QualType ElemType, APValue const &VecVal1,
                                 APValue const &VecVal2, unsigned EltNum,
                                 APValue &Result) {
-  unsigned const TotalElementsInAVector = VecVal1.getVectorLength();
-
-  Expr const *IndexExpr = E->getExpr(2 + EltNum);
-  APSInt IndexVal;
-  if (!EvaluateInteger(IndexExpr, IndexVal, Info))
-    return false;
+  unsigned const TotalElementsInInputVector = VecVal1.getVectorLength();
 
-  uint32_t index = IndexVal.getZExtValue();
+  APSInt IndexVal = E->getShuffleMaskIdx(Info.Ctx, EltNum);
+  int64_t index = IndexVal.getExtValue();
   // The spec says that -1 should be treated as undef for optimizations,
   // but in constexpr we need to choose a value. We'll choose 0.
-  if (index >= TotalElementsInAVector * 2)
-    index = 0;
+  if (index == -1) {
+    Info.FFDiag(
+        E, diag::err_shufflevector_minus_one_is_undefined_behavior_constexpr)
+        << EltNum;
+    return false;
+  }
+
+  if (index < 0 || index >= TotalElementsInInputVector * 2)
+    llvm_unreachable("Out of bounds shuffle index");
 
-  if (index >= TotalElementsInAVector)
-    Result = VecVal2.getVectorElt(index - TotalElementsInAVector);
+  if (index >= TotalElementsInInputVector)
+    Result = VecVal2.getVectorElt(index - TotalElementsInInputVector);
   else
     Result = VecVal1.getVectorElt(index);
   return true;
@@ -11064,11 +11053,11 @@ static bool handleVectorShuffle(EvalInfo &Info, const ShuffleVectorExpr *E,
 bool VectorExprEvaluator::VisitShuffleVectorExpr(const ShuffleVectorExpr *E) {
   APValue VecVal1;
   const Expr *Vec1 = E->getExpr(0);
-  if (!EvaluateVectorOrLValue(VecVal1, Info, Vec1, Vec1->getType()))
+  if (!EvaluateAsRValue(Info, Vec1, VecVal1))
     return false;
   APValue VecVal2;
   const Expr *Vec2 = E->getExpr(1);
-  if (!EvaluateVectorOrLValue(VecVal2, Info, Vec2, Vec2->getType()))
+  if (!EvaluateAsRValue(Info, Vec2, VecVal2))
     return false;
 
   VectorType const *DestVecTy = E->getType()->castAs<VectorType>();

clang/test/Sema/constant_builtins_vector.cpp

@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -verify -std=c++2a -fsyntax-only %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -verify -std=c++2a -fsyntax-only -Wno-bit-int-extension %s
 
 #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
 #define LITTLE_END 1
@@ -9,18 +8,29 @@
 #error "huh?"
 #endif
 
+// We also support _BitInt as long as it is >=8 and a power of 2.
+typedef _BitInt(8) BitInt8;
+typedef _BitInt(32) BitInt32;
+typedef _BitInt(128) BitInt128;
+
 typedef double vector4double __attribute__((__vector_size__(32)));
 typedef float vector4float __attribute__((__vector_size__(16)));
 typedef long long vector4long __attribute__((__vector_size__(32)));
 typedef int vector4int __attribute__((__vector_size__(16)));
 typedef short vector4short __attribute__((__vector_size__(8)));
 typedef char vector4char __attribute__((__vector_size__(4)));
+typedef BitInt8 vector4BitInt8 __attribute__((__vector_size__(4)));
+typedef BitInt32 vector4BitInt32 __attribute__((__vector_size__(16)));
+typedef BitInt128 vector4BitInt128 __attribute__((__vector_size__(64)));
 typedef double vector8double __attribute__((__vector_size__(64)));
 typedef float vector8float __attribute__((__vector_size__(32)));
 typedef long long vector8long __attribute__((__vector_size__(64)));
 typedef int vector8int __attribute__((__vector_size__(32)));
 typedef short vector8short __attribute__((__vector_size__(16)));
 typedef char vector8char __attribute__((__vector_size__(8)));
+typedef BitInt8 vector8BitInt8 __attribute__((__vector_size__(8)));
+typedef BitInt32 vector8BitInt32 __attribute__((__vector_size__(32)));
+typedef BitInt128 vector8BitInt128 __attribute__((__vector_size__(128)));
 
 #define CHECK_NUM(__size, __typeFrom, __typeTo, ...)                            \
   constexpr vector##__size##__typeTo                                            \
@@ -34,6 +44,9 @@ typedef char vector8char __attribute__((__vector_size__(8)));
   CHECK_NUM(__size, __typeFrom, int, __VA_ARGS__)                              \
   CHECK_NUM(__size, __typeFrom, short, __VA_ARGS__)                            \
   CHECK_NUM(__size, __typeFrom, char, __VA_ARGS__)                             \
+  CHECK_NUM(__size, __typeFrom, BitInt8, __VA_ARGS__)                          \
+  CHECK_NUM(__size, __typeFrom, BitInt32, __VA_ARGS__)                         \
+  CHECK_NUM(__size, __typeFrom, BitInt128, __VA_ARGS__)                        \
   static_assert(                                                               \
       __builtin_bit_cast(                                                      \
           unsigned,                                                            \
@@ -56,7 +69,10 @@ typedef char vector8char __attribute__((__vector_size__(8)));
   CHECK_TO_ALL_TYPES(__size, long, __VA_ARGS__)                                \
   CHECK_TO_ALL_TYPES(__size, int, __VA_ARGS__)                                 \
   CHECK_TO_ALL_TYPES(__size, short, __VA_ARGS__)                               \
-  CHECK_TO_ALL_TYPES(__size, char, __VA_ARGS__)
+  CHECK_TO_ALL_TYPES(__size, char, __VA_ARGS__)                                \
+  CHECK_TO_ALL_TYPES(__size, BitInt8, __VA_ARGS__)                             \
+  CHECK_TO_ALL_TYPES(__size, BitInt32, __VA_ARGS__)                            \
+  CHECK_TO_ALL_TYPES(__size, BitInt128, __VA_ARGS__)
 
 CHECK_ALL_COMBINATIONS(4, 0, 1, 2, 3);
 CHECK_ALL_COMBINATIONS(8, 0, 1, 2, 3, 4, 5, 6, 7);
@@ -77,23 +93,28 @@ constexpr vector4char vectorShuffle2 =
     __builtin_shufflevector(vector4charConst1, vector4charConst2, 4, 5, 6, 7);
 static_assert(__builtin_bit_cast(unsigned, vectorShuffle2) ==
               (LITTLE_END ? 0x07060504 : 0x04050607));
-constexpr vector4char vectorShuffle3 = __builtin_shufflevector(
-    vector4charConst1, vector4charConst2, -1, -1, -1, -1);
-static_assert(__builtin_bit_cast(unsigned, vectorShuffle3) ==
-              (LITTLE_END ? 0x00000000 : 0x00000000));
-constexpr vector4char vectorShuffle4 =
+constexpr vector4char vectorShuffle3 =
     __builtin_shufflevector(vector4charConst1, vector4charConst2, 0, 2, 4, 6);
-static_assert(__builtin_bit_cast(unsigned, vectorShuffle4) ==
+static_assert(__builtin_bit_cast(unsigned, vectorShuffle3) ==
               (LITTLE_END ? 0x06040200 : 0x00020406));
-constexpr vector8char vectorShuffle5 = __builtin_shufflevector(
+constexpr vector8char vectorShuffle4 = __builtin_shufflevector(
     vector8intConst, vector8intConst, 0, 2, 4, 6, 8, 10, 12, 14);
-static_assert(__builtin_bit_cast(unsigned long long, vectorShuffle5) ==
+static_assert(__builtin_bit_cast(unsigned long long, vectorShuffle4) ==
               (LITTLE_END ? 0x0E0C0A080E0C0A08 : 0x080A0C0E080A0C0E));
-constexpr vector4char vectorShuffle6 =
+constexpr vector4char vectorShuffle5 =
     __builtin_shufflevector(vector8intConst, vector8intConst, 0, 2, 4, 6);
-static_assert(__builtin_bit_cast(unsigned, vectorShuffle6) ==
+static_assert(__builtin_bit_cast(unsigned, vectorShuffle5) ==
               (LITTLE_END ? 0x0E0C0A08 : 0x080A0C0E));
-constexpr vector8char vectorShuffle7 = __builtin_shufflevector(
+constexpr vector8char vectorShuffle6 = __builtin_shufflevector(
     vector4charConst1, vector4charConst2, 0, 2, 4, 6, 1, 3, 5, 7);
-static_assert(__builtin_bit_cast(unsigned long long, vectorShuffle7) ==
+static_assert(__builtin_bit_cast(unsigned long long, vectorShuffle6) ==
               (LITTLE_END ? 0x0705030106040200 : 0x0002040601030507));
+
+constexpr vector4char
+    vectorShuffleFail1 = // expected-error {{constexpr variable 'vectorShuffleFail1'\
+ must be initialized by a constant expression}}
+    __builtin_shufflevector( // expected-error {{index for __builtin_shufflevector must be within\
+ the bounds of the input vectors in a constexpr context. An index of\
+ -1 at position 0 was found. -1 is only allowed at runtime.}}
+        vector4charConst1,
+        vector4charConst2, -1, -1, -1, -1);

clang/test/Sema/convertvector.c

@@ -15,3 +15,6 @@ vector8float foo3(double x) {
   return __builtin_convertvector(x, vector8float);  // expected-error {{must be a vector}}
 }
 
+float foo4(float x) {
+  return __builtin_convertvector(x, float); // expected-error {{first argument to __builtin_convertvector must be a vector}}
+}