Skip to content

[clang][bytecode] Implement arithmetic, bitwise and compound assignment operator #108949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 21, 2024

Conversation

yronglin
Copy link
Contributor

@yronglin yronglin commented Sep 17, 2024

Implement +, -, *, / , %, &, |, ^, <<, >> and compound assignment operator.

@yronglin yronglin requested a review from tbaederr September 17, 2024 09:44
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2024

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

Implement +, -, *, / , %, &amp;, |, ^, &lt;&lt;, &gt;&gt; and compound assignment operator.


Patch is 35.67 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/108949.diff

3 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Compiler.cpp (+112-9)
  • (modified) clang/test/AST/ByteCode/constexpr-vectors.cpp (+498-21)
  • (modified) clang/test/SemaCXX/constexpr-vectors.cpp (+1)
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 7e0775a51aee61..e7a6df58e6f1a6 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -1267,12 +1267,8 @@ bool Compiler<Emitter>::VisitVectorBinOp(const BinaryOperator *E) {
   assert(E->getLHS()->getType()->isVectorType());
   assert(E->getRHS()->getType()->isVectorType());
 
-  // FIXME: Current only support comparison binary operator, add support for
-  // other binary operator.
-  if (!E->isComparisonOp() && !E->isLogicalOp())
-    return this->emitInvalid(E);
   // Prepare storage for result.
-  if (!Initializing) {
+  if (!Initializing || E->isCompoundAssignmentOp()) {
     unsigned LocalIndex = allocateTemporary(E);
     if (!this->emitGetPtrLocal(LocalIndex, E))
       return false;
@@ -1281,26 +1277,67 @@ bool Compiler<Emitter>::VisitVectorBinOp(const BinaryOperator *E) {
   const Expr *LHS = E->getLHS();
   const Expr *RHS = E->getRHS();
   const auto *VecTy = E->getType()->getAs<VectorType>();
+  auto Op = E->isCompoundAssignmentOp()
+                ? BinaryOperator::getOpForCompoundAssignment(E->getOpcode())
+                : E->getOpcode();
 
   // The LHS and RHS of a comparison operator must have the same type. So we
   // just use LHS vector element type here.
   PrimType ElemT = this->classifyVectorElementType(LHS->getType());
   PrimType ResultElemT = this->classifyVectorElementType(E->getType());
 
-  // Evaluate LHS and save value to LHSOffset.
+  // Allocate a local pointer for LHS and RHS.
   unsigned LHSOffset = this->allocateLocalPrimitive(LHS, PT_Ptr, true, false);
+  unsigned RHSOffset = this->allocateLocalPrimitive(RHS, PT_Ptr, true, false);
+
+  // C++17 onwards require that we evaluate the RHS of the compound
+  // assignment op first.
+  if (E->isCompoundAssignmentOp()) {
+    // Evaluate RHS and save value to RHSOffset.
+    if (!this->visit(RHS))
+      return false;
+    if (!this->emitSetLocal(PT_Ptr, RHSOffset, E))
+      return false;
+
+    // Evaluate LHS and save value to LHSOffset.
+    if (!this->visit(LHS))
+      return false;
+    if (!this->emitSetLocal(PT_Ptr, LHSOffset, E))
+      return false;
+  } else {
+    // Evaluate LHS and save value to LHSOffset.
+    if (!this->visit(LHS))
+      return false;
+    if (!this->emitSetLocal(PT_Ptr, LHSOffset, E))
+      return false;
+
+    // Evaluate RHS and save value to RHSOffset.
+    if (!this->visit(RHS))
+      return false;
+    if (!this->emitSetLocal(PT_Ptr, RHSOffset, E))
+      return false;
+  }
+
+  // Evaluate LHS and save value to LHSOffset.
   if (!this->visit(LHS))
     return false;
   if (!this->emitSetLocal(PT_Ptr, LHSOffset, E))
     return false;
 
   // Evaluate RHS and save value to RHSOffset.
-  unsigned RHSOffset = this->allocateLocalPrimitive(RHS, PT_Ptr, true, false);
   if (!this->visit(RHS))
     return false;
   if (!this->emitSetLocal(PT_Ptr, RHSOffset, E))
     return false;
 
+  // BitAdd/BitOr/BitXor/Shl/Shr doesn't support bool type, we need perform the
+  // integer promotion.
+  bool NeedIntPromot = ElemT == PT_Bool && (E->isBitwiseOp() || E->isShiftOp());
+  QualType PromotTy =
+      Ctx.getASTContext().getPromotedIntegerType(Ctx.getASTContext().BoolTy);
+  PrimType PromotT = classifyPrim(PromotTy);
+  PrimType OpT = NeedIntPromot ? PromotT : ElemT;
+
   auto getElem = [=](unsigned Offset, unsigned Index) {
     if (!this->emitGetLocal(PT_Ptr, Offset, E))
       return false;
@@ -1311,16 +1348,63 @@ bool Compiler<Emitter>::VisitVectorBinOp(const BinaryOperator *E) {
         return false;
       if (!this->emitPrimCast(PT_Bool, ResultElemT, VecTy->getElementType(), E))
         return false;
+    } else if (NeedIntPromot) {
+      if (!this->emitPrimCast(ElemT, PromotT, PromotTy, E))
+        return false;
     }
     return true;
   };
 
+#define EMIT_ARITH_OP(OP)                                                      \
+  {                                                                            \
+    if (ElemT == PT_Float) {                                                   \
+      if (!this->emit##OP##f(getFPOptions(E), E))                              \
+        return false;                                                          \
+    } else {                                                                   \
+      if (!this->emit##OP(ElemT, E))                                           \
+        return false;                                                          \
+    }                                                                          \
+    break;                                                                     \
+  }
+
   for (unsigned I = 0; I != VecTy->getNumElements(); ++I) {
     if (!getElem(LHSOffset, I))
       return false;
     if (!getElem(RHSOffset, I))
       return false;
-    switch (E->getOpcode()) {
+    switch (Op) {
+    case BO_Add:
+      EMIT_ARITH_OP(Add)
+    case BO_Sub:
+      EMIT_ARITH_OP(Sub)
+    case BO_Mul:
+      EMIT_ARITH_OP(Mul)
+    case BO_Div:
+      EMIT_ARITH_OP(Div)
+    case BO_Rem:
+      if (!this->emitRem(ElemT, E))
+        return false;
+      break;
+    case BO_And:
+      if (!this->emitBitAnd(OpT, E))
+        return false;
+      break;
+    case BO_Or:
+      if (!this->emitBitOr(OpT, E))
+        return false;
+      break;
+    case BO_Xor:
+      if (!this->emitBitXor(OpT, E))
+        return false;
+      break;
+    case BO_Shl:
+      if (!this->emitShl(OpT, ElemT, E))
+        return false;
+      break;
+    case BO_Shr:
+      if (!this->emitShr(OpT, ElemT, E))
+        return false;
+      break;
     case BO_EQ:
       if (!this->emitEQ(ElemT, E))
         return false;
@@ -1356,7 +1440,7 @@ bool Compiler<Emitter>::VisitVectorBinOp(const BinaryOperator *E) {
         return false;
       break;
     default:
-      llvm_unreachable("Unsupported binary operator");
+      return this->emitInvalid(E);
     }
 
     // The result of the comparison is a vector of the same width and number
@@ -1371,10 +1455,27 @@ bool Compiler<Emitter>::VisitVectorBinOp(const BinaryOperator *E) {
         return false;
     }
 
+    // If we performed an integer promotion, we need to cast the compute result
+    // into result vector element type.
+    if (NeedIntPromot &&
+        !this->emitPrimCast(PromotT, ResultElemT, VecTy->getElementType(), E))
+      return false;
+
     // Initialize array element with the value we just computed.
     if (!this->emitInitElem(ResultElemT, I, E))
       return false;
   }
+
+  if (E->isCompoundAssignmentOp()) {
+    if (!this->emitGetLocal(PT_Ptr, LHSOffset, E))
+      return false;
+    if (!this->emitFlip(PT_Ptr, PT_Ptr, E))
+      return false;
+    if (!this->emitMemcpy(E))
+      return false;
+    if (!this->emitPopPtr(E))
+      return false;
+  }
   return true;
 }
 
@@ -2292,6 +2393,8 @@ bool Compiler<Emitter>::VisitPointerCompoundAssignOperator(
 template <class Emitter>
 bool Compiler<Emitter>::VisitCompoundAssignOperator(
     const CompoundAssignOperator *E) {
+  if (E->getType()->isVectorType())
+    return VisitVectorBinOp(E);
 
   const Expr *LHS = E->getLHS();
   const Expr *RHS = E->getRHS();
diff --git a/clang/test/AST/ByteCode/constexpr-vectors.cpp b/clang/test/AST/ByteCode/constexpr-vectors.cpp
index 7a65b263784586..629cdd0bacb5b6 100644
--- a/clang/test/AST/ByteCode/constexpr-vectors.cpp
+++ b/clang/test/AST/ByteCode/constexpr-vectors.cpp
@@ -10,11 +10,217 @@ using FourI128VecSize __attribute__((vector_size(64))) = __int128;
 
 using FourCharsExtVec __attribute__((ext_vector_type(4))) = char;
 using FourIntsExtVec __attribute__((ext_vector_type(4))) = int;
+using FourLongLongsExtVec __attribute__((ext_vector_type(4))) = long long;
+using FourFloatsExtVec __attribute__((ext_vector_type(4))) = float;
+using FourDoublesExtVec __attribute__((ext_vector_type(4))) = double;
 using FourI128ExtVec __attribute__((ext_vector_type(4))) = __int128;
 
+
+// Next a series of tests to make sure these operations are usable in
+// constexpr functions. Template instantiations don't emit Winvalid-constexpr,
+// so we have to do these as macros.
+#define MathShiftOps(Type)                            \
+  constexpr auto MathShiftOps##Type(Type a, Type b) { \
+    a = a + b;                                        \
+    a = a - b;                                        \
+    a = a * b;                                        \
+    a = a / b;                                        \
+    b = a + 1;                                        \
+    b = a - 1;                                        \
+    b = a * 1;                                        \
+    b = a / 1;                                        \
+    a += a;                                           \
+    a -= a;                                           \
+    a *= a;                                           \
+    a /= a;                                           \
+    b += a;                                           \
+    b -= a;                                           \
+    b *= a;                                           \
+    b /= a;                                           \
+    a < b;                                            \
+    a > b;                                            \
+    a <= b;                                           \
+    a >= b;                                           \
+    a == b;                                           \
+    a != b;                                           \
+    a &&b;                                            \
+    a || b;                                           \
+    auto c = (a, b);                                  \
+    return c;                                         \
+  }
+
+// Ops specific to Integers.
+#define MathShiftOpsInts(Type)                            \
+  constexpr auto MathShiftopsInts##Type(Type a, Type b) { \
+    a = a << b;                                           \
+    a = a >> b;                                           \
+    a = a << 3;                                           \
+    a = a >> 3;                                           \
+    a = 3 << b;                                           \
+    a = 3 >> b;                                           \
+    a <<= b;                                              \
+    a >>= b;                                              \
+    a <<= 3;                                              \
+    a >>= 3;                                              \
+    a = a % b;                                            \
+    a &b;                                                 \
+    a | b;                                                \
+    a ^ b;                                                \
+    return a;                                             \
+  }
+
+MathShiftOps(FourCharsVecSize);
+MathShiftOps(FourIntsVecSize);
+MathShiftOps(FourLongLongsVecSize);
+MathShiftOps(FourFloatsVecSize);
+MathShiftOps(FourDoublesVecSize);
+MathShiftOps(FourCharsExtVec);
+MathShiftOps(FourIntsExtVec);
+MathShiftOps(FourLongLongsExtVec);
+MathShiftOps(FourFloatsExtVec);
+MathShiftOps(FourDoublesExtVec);
+
+MathShiftOpsInts(FourCharsVecSize);
+MathShiftOpsInts(FourIntsVecSize);
+MathShiftOpsInts(FourLongLongsVecSize);
+MathShiftOpsInts(FourCharsExtVec);
+MathShiftOpsInts(FourIntsExtVec);
+MathShiftOpsInts(FourLongLongsExtVec);
+
+template <typename T, typename U>
+constexpr auto CmpMul(T t, U u) {
+  t *= u;
+  return t;
+}
+template <typename T, typename U>
+constexpr auto CmpDiv(T t, U u) {
+  t /= u;
+  return t;
+}
+template <typename T, typename U>
+constexpr auto CmpRem(T t, U u) {
+  t %= u;
+  return t;
+}
+
+template <typename T, typename U>
+constexpr auto CmpAdd(T t, U u) {
+  t += u;
+  return t;
+}
+
+template <typename T, typename U>
+constexpr auto CmpSub(T t, U u) {
+  t -= u;
+  return t;
+}
+
+template <typename T, typename U>
+constexpr auto CmpLSH(T t, U u) {
+  t <<= u;
+  return t;
+}
+
+template <typename T, typename U>
+constexpr auto CmpRSH(T t, U u) {
+  t >>= u;
+  return t;
+}
+
+template <typename T, typename U>
+constexpr auto CmpBinAnd(T t, U u) {
+  t &= u;
+  return t;
+}
+
+template <typename T, typename U>
+constexpr auto CmpBinXOr(T t, U u) {
+  t ^= u;
+  return t;
+}
+
+template <typename T, typename U>
+constexpr auto CmpBinOr(T t, U u) {
+  t |= u;
+  return t;
+}
+
+constexpr auto CmpF(float t, float u) {
+  return __builtin_fabs(t - u) < 0.0001;
+}
+
 // Only int vs float makes a difference here, so we only need to test 1 of each.
 // Test Char to make sure the mixed-nature of shifts around char is evident.
 void CharUsage() {
+  constexpr auto a = FourCharsVecSize{6, 3, 2, 1} +
+            FourCharsVecSize{12, 15, 5, 7};
+  static_assert(a[0] == 18 && a[1] == 18 && a[2] == 7 && a[3] == 8, "");
+
+  constexpr auto b = FourCharsVecSize{19, 15, 13, 12} -
+                     FourCharsVecSize{13, 14, 5, 3};
+  static_assert(b[0] == 6 && b[1] == 1 && b[2] == 8 && b[3] == 9, "");
+
+  constexpr auto c = FourCharsVecSize{8, 4, 2, 1} *
+                     FourCharsVecSize{3, 4, 5, 6};
+  static_assert(c[0] == 24 && c[1] == 16 && c[2] == 10 && c[3] == 6, "");
+
+  constexpr auto d = FourCharsVecSize{12, 12, 10, 10} /
+                     FourCharsVecSize{6, 4, 5, 2};
+  static_assert(d[0] == 2 && d[1] == 3 && d[2] == 2 && d[3] == 5, "");
+
+  constexpr auto e = FourCharsVecSize{12, 12, 10, 10} %
+                     FourCharsVecSize{6, 4, 4, 3};
+  static_assert(e[0] == 0 && e[1] == 0 && e[2] == 2 && e[3] == 1, "");
+
+  constexpr auto f = FourCharsVecSize{6, 3, 2, 1} + 3;
+  static_assert(f[0] == 9 && f[1] == 6 && f[2] == 5 && f[3] == 4, "");
+
+  constexpr auto g = FourCharsVecSize{19, 15, 12, 10} - 3;
+  static_assert(g[0] == 16 && g[1] == 12 && g[2] == 9 && g[3] == 7, "");
+
+  constexpr auto h = FourCharsVecSize{8, 4, 2, 1} * 3;
+  static_assert(h[0] == 24 && h[1] == 12 && h[2] == 6 && h[3] == 3, "");
+
+  constexpr auto j = FourCharsVecSize{12, 15, 18, 21} / 3;
+  static_assert(j[0] == 4 && j[1] == 5 && j[2] == 6 && j[3] == 7, "");
+
+  constexpr auto k = FourCharsVecSize{12, 17, 19, 22} % 3;
+  static_assert(k[0] == 0 && k[1] == 2 && k[2] == 1 && k[3] == 1, "");
+
+  constexpr auto l = 3 + FourCharsVecSize{6, 3, 2, 1};
+  static_assert(l[0] == 9 && l[1] == 6 && l[2] == 5 && l[3] == 4, "");
+
+  constexpr auto m = 20 - FourCharsVecSize{19, 15, 12, 10};
+  static_assert(m[0] == 1 && m[1] == 5 && m[2] == 8 && m[3] == 10, "");
+
+  constexpr auto n = 3 * FourCharsVecSize{8, 4, 2, 1};
+  static_assert(n[0] == 24 && n[1] == 12 && n[2] == 6 && n[3] == 3, "");
+
+  constexpr auto o = 100 / FourCharsVecSize{12, 15, 18, 21};
+  static_assert(o[0] == 8 && o[1] == 6 && o[2] == 5 && o[3] == 4, "");
+
+  constexpr auto p = 100 % FourCharsVecSize{12, 15, 18, 21};
+  static_assert(p[0] == 4 && p[1] == 10 && p[2] == 10 && p[3] == 16, "");
+
+  constexpr auto q = FourCharsVecSize{6, 3, 2, 1} << FourCharsVecSize{1, 1, 2, 2};
+  static_assert(q[0] == 12 && q[1] == 6 && q[2] == 8 && q[3] == 4, "");
+
+  constexpr auto r = FourCharsVecSize{19, 15, 12, 10} >>
+                     FourCharsVecSize{1, 1, 2, 2};
+  static_assert(r[0] == 9 && r[1] == 7 && r[2] == 3 && r[3] == 2, "");
+
+  constexpr auto s = FourCharsVecSize{6, 3, 5, 10} << 1;
+  static_assert(s[0] == 12 && s[1] == 6 && s[2] == 10 && s[3] == 20, "");
+
+  constexpr auto t = FourCharsVecSize{19, 15, 10, 20} >> 1;
+  static_assert(t[0] == 9 && t[1] == 7 && t[2] == 5 && t[3] == 10, "");
+
+  constexpr auto u = 12 << FourCharsVecSize{1, 2, 3, 3};
+  static_assert(u[0] == 24 && u[1] == 48 && u[2] == 96 && u[3] == 96, "");
+
+  constexpr auto v = 12 >> FourCharsVecSize{1, 2, 2, 1};
+  static_assert(v[0] == 6 && v[1] == 3 && v[2] == 3 && v[3] == 6, "");
+
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
                      FourCharsVecSize{4, 3, 2, 1};
   static_assert(w[0] == -1 && w[1] == -1 && w[2] == 0 && w[3] == 0, "");
@@ -57,6 +263,27 @@ void CharUsage() {
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
   static_assert(H[0] == -1 && H[1] == -1 && H[2] == 0 && H[3] == -1, "");
 
+  constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
+                     FourCharsVecSize{4, 3, 2, 1};
+  static_assert(I[0] == 0 && I[1] == 2 && I[2] == 2 && I[3] == 0, "");
+
+  constexpr auto J = FourCharsVecSize{1, 2, 3, 4} ^
+                     FourCharsVecSize { 4, 3, 2, 1 };
+  static_assert(J[0] == 5 && J[1] == 1 && J[2] == 1 && J[3] == 5, "");
+
+  constexpr auto K = FourCharsVecSize{1, 2, 3, 4} |
+                     FourCharsVecSize{4, 3, 2, 1};
+  static_assert(K[0] == 5 && K[1] == 3 && K[2] == 3 && K[3] == 5, "");
+
+  constexpr auto L = FourCharsVecSize{1, 2, 3, 4} & 3;
+  static_assert(L[0] == 1 && L[1] == 2 && L[2] == 3 && L[3] == 0, "");
+
+  constexpr auto M = FourCharsVecSize{1, 2, 3, 4} ^ 3;
+  static_assert(M[0] == 2 && M[1] == 1 && M[2] == 0 && M[3] == 7, "");
+
+  constexpr auto N = FourCharsVecSize{1, 2, 3, 4} | 3;
+  static_assert(N[0] == 3 && N[1] == 3 && N[2] == 3 && N[3] == 7, "");
+
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
                      FourCharsVecSize{5, 5, 0, 0};
   static_assert(O[0] == 1 && O[1] == 0 && O[2] == 0 && O[3] == 0, "");
@@ -71,10 +298,39 @@ void CharUsage() {
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
   static_assert(R[0] == 1 && R[1] == 1 && R[2] == 1 && R[3] == 1, "");
 
-  constexpr auto H1 = FourCharsVecSize{-1, -1, 0, -1};
-  constexpr auto InvH = -H1;
+  constexpr auto T = CmpMul(a, b);
+  static_assert(T[0] == 108 && T[1] == 18 && T[2] == 56 && T[3] == 72, "");
+
+  constexpr auto U = CmpDiv(a, b);
+  static_assert(U[0] == 3 && U[1] == 18 && U[2] == 0 && U[3] == 0, "");
+
+  constexpr auto V = CmpRem(a, b);
+  static_assert(V[0] == 0 && V[1] == 0 && V[2] == 7 && V[3] == 8, "");
+
+  constexpr auto X = CmpAdd(a, b);
+  static_assert(X[0] == 24 && X[1] == 19 && X[2] == 15 && X[3] == 17, "");
+
+  constexpr auto Y = CmpSub(a, b);
+  static_assert(Y[0] == 12 && Y[1] == 17 && Y[2] == -1 && Y[3] == -1, "");
+
+  constexpr auto InvH = -H;
   static_assert(InvH[0] == 1 && InvH[1] == 1 && InvH[2] == 0 && InvH[3] == 1, "");
 
+  constexpr auto Z = CmpLSH(a, InvH);
+  static_assert(Z[0] == 36 && Z[1] == 36 && Z[2] == 7 && Z[3] == 16, "");
+
+  constexpr auto aa = CmpRSH(a, InvH);
+  static_assert(aa[0] == 9 && aa[1] == 9 && aa[2] == 7 && aa[3] == 4, "");
+
+  constexpr auto ab = CmpBinAnd(a, b);
+  static_assert(ab[0] == 2 && ab[1] == 0 && ab[2] == 0 && ab[3] == 8, "");
+
+  constexpr auto ac = CmpBinXOr(a, b);
+  static_assert(ac[0] == 20 && ac[1] == 19 && ac[2] == 15 && ac[3] == 1, "");
+
+  constexpr auto ad = CmpBinOr(a, b);
+  static_assert(ad[0] == 22 && ad[1] == 19 && ad[2] == 15 && ad[3] == 9, "");
+
   constexpr auto ae = ~FourCharsVecSize{1, 2, 10, 20};
   static_assert(ae[0] == -2 && ae[1] == -3 && ae[2] == -11 && ae[3] == -21, "");
 
@@ -83,6 +339,75 @@ void CharUsage() {
 }
 
 void CharExtVecUsage() {
+  constexpr auto a = FourCharsExtVec{6, 3, 2, 1} +
+                     FourCharsExtVec{12, 15, 5, 7};
+  static_assert(a[0] == 18 && a[1] == 18 && a[2] == 7 && a[3] == 8, "");
+
+  constexpr auto b = FourCharsExtVec{19, 15, 13, 12} -
+                     FourCharsExtVec{13, 14, 5, 3};
+  static_assert(b[0] == 6 && b[1] == 1 && b[2] == 8 && b[3] == 9, "");
+
+  constexpr auto c = FourCharsExtVec{8, 4, 2, 1} *
+                     FourCharsExtVec{3, 4, 5, 6};
+  static_assert(c[0] == 24 && c[1] == 16 && c[2] == 10 && c[3] == 6, "");
+
+  constexpr auto d = FourCharsExtVec{12, 12, 10, 10} /
+                     FourCharsExtVec{6, 4, 5, 2};
+  static_assert(d[0] == 2 && d[1] == 3 && d[2] == 2 && d[3] == 5, "");
+
+  constexpr auto e = FourCharsExtVec{12, 12, 10, 10} %
+                     FourCharsExtVec{6, 4, 4, 3};
+  static_assert(e[0] == 0 && e[1] == 0 && e[2] == 2 && e[3] == 1, "");
+
+  constexpr auto f = FourCharsExtVec{6, 3, 2, 1} + 3;
+  static_assert(f[0] == 9 && f[1] == 6 && f[2] == 5 && f[3] == 4, "");
+
+  constexpr auto g = FourCharsExtVec{19, 15, 12, 10} - 3;
+  static_assert(g[0] == 16 && g[1] == 12 && g[2] == 9 && g[3] == 7, "");
+
+  constexpr auto h = FourCharsExtVec{8, 4, 2, 1} * 3;
+  static_assert(h[0] == 24 && h[1] == 12 && h[2] == 6 && h[3] == 3, "");
+
+  constexpr auto j = FourCharsExtVec{12, 15, 18, 21} / 3;
+  static_assert(j[0] == 4 && j[1] == 5 && j[2] == 6 && j[3] == 7, "");
+
+  constexpr auto k = FourCharsExtVec{...
[truncated]

@yronglin
Copy link
Contributor Author

Thanks for your review!

@tbaederr
Copy link
Contributor

Does it work to not discard the result of a compound operator?

using VI __attribute__((ext_vector_type(4))) = int;

constexpr int a() {
    VI a = {0, 0, 0, 0};
    VI b = {1,1,1,1};

    VI C = (a += b);

    return 0;
}

static_assert(a() == 0);

…d assignment operator was not discarded

Signed-off-by: yronglin <[email protected]>
@yronglin
Copy link
Contributor Author

Does it work to not discard the result of a compound operator?

using VI __attribute__((ext_vector_type(4))) = int;

constexpr int a() {
    VI a = {0, 0, 0, 0};
    VI b = {1,1,1,1};

    VI C = (a += b);

    return 0;
}

static_assert(a() == 0);

Good catch! I've fixed this issue.

@tbaederr
Copy link
Contributor

Same question for regular binary operators:

using VI __attribute__((ext_vector_type(4))) = int;

constexpr int a() {
    VI a = {0, 0, 0, 0};
    VI b = {1,1,1,1};

    VI C = (a + b);

    return 0;
}

static_assert(a() == 0);

Also, might make sense to add them to test/AST/ByteCode/vectors.cpp.

@yronglin
Copy link
Contributor Author

Sure, added them to test. And regular binary operators works well.

@yronglin yronglin merged commit f5a65d8 into llvm:main Sep 21, 2024
8 checks passed
@rorth
Copy link
Collaborator

rorth commented Sep 21, 2024

This patch broke the Solaris/sparcv9 buildbot. The two affected tests use -triple x86_64-linux-gnu without requiring x86 support.

@tbaederr
Copy link
Contributor

Is x86-solaris11-sparcv9 the right triple to use to reproduce issues for that builder?

@rorth
Copy link
Collaborator

rorth commented Sep 21, 2024

No, that doesn't exist. The failure will occur on any non-x86 build configure with -DLLVM_TARGETS_TO_BUILD=<anything not including X86>.

AFAICS the tests just need

// REQUIRES: x86-registered-target

@tbaederr
Copy link
Contributor

But if that was the entire problem, the one in test/SemaCXX/ would've failed before or not?

@rorth
Copy link
Collaborator

rorth commented Sep 21, 2024

Seems so, yes. Could also be an endianess thing. I know nothing about this test, though.

@rorth
Copy link
Collaborator

rorth commented Sep 23, 2024

This is weird: I'd initially tried both all-targets and sparc-only builds, and in neither case did the failures occur. However, those were 2-stage builds (as I usually use). When I switched to a 1-stage build with clang-19 as build compiler, I get the same failure as on the bot locally, so this isn't just a quirk of the bot build environment.

@tbaederr
Copy link
Contributor

@yronglin Could you try to investigate?

@rorth
Copy link
Collaborator

rorth commented Sep 23, 2024

Just for the record: in a 2-stage sparc64-unknown-linux-gnu build, the failures occur, too. So nothing Solaris-specific in here. Maybe endianess?

@tbaederr
Copy link
Contributor

Endianness is very well possible indeed, but it's a little weird this starts failing now, since the shifts done aren't vector specific.
Are you testing this on a solaris host?
When you say stage2-build, you mean you're building clang main with your system compiler (this is stage1, and the tests pass here?) and then use that compiler to build the same clang main again, but this time the tests fail?

@nikic
Copy link
Contributor

nikic commented Sep 24, 2024

We're seeing failures on s390x as well (which is big endian):


******************** TEST 'Clang :: SemaCXX/constexpr-vectors.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/../lib/clang/20/include -nostdsysteminc /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp -triple x86_64-linux-gnu -Wno-uninitialized -std=c++14 -fsyntax-only -verify
+ /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/../lib/clang/20/include -nostdsysteminc /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp -triple x86_64-linux-gnu -Wno-uninitialized -std=c++14 -fsyntax-only -verify
RUN: at line 2: /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/../lib/clang/20/include -nostdsysteminc /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp -triple x86_64-linux-gnu -Wno-uninitialized -std=c++14 -fsyntax-only -verify -fexperimental-new-constant-interpreter
+ /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/../lib/clang/20/include -nostdsysteminc /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp -triple x86_64-linux-gnu -Wno-uninitialized -std=c++14 -fsyntax-only -verify -fexperimental-new-constant-interpreter
error: 'expected-error' diagnostics seen but not expected: 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp Line 215: static assertion failed due to requirement 's[0] == 12': 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp Line 218: static assertion failed due to requirement 't[0] == 9': 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp Line 402: static assertion failed due to requirement 's[0] == 12': 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp Line 405: static assertion failed due to requirement 't[0] == 9': 
error: 'expected-note' diagnostics seen but not expected: 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp Line 215: expression evaluates to ''<U+0006>' (0x06, 6) == 12'
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp Line 218: expression evaluates to ''<U+0013>' (0x13, 19) == 9'
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp Line 402: expression evaluates to ''<U+0006>' (0x06, 6) == 12'
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/SemaCXX/constexpr-vectors.cpp Line 405: expression evaluates to ''<U+0013>' (0x13, 19) == 9'
8 errors generated.

--

********************


******************** TEST 'Clang :: AST/ByteCode/constexpr-vectors.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/../lib/clang/20/include -nostdsysteminc /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp -triple x86_64-linux-gnu -std=c++14 -fsyntax-only -verify
+ /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/../lib/clang/20/include -nostdsysteminc /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp -triple x86_64-linux-gnu -std=c++14 -fsyntax-only -verify
RUN: at line 2: /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/../lib/clang/20/include -nostdsysteminc /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp -triple x86_64-linux-gnu -fexperimental-new-constant-interpreter -std=c++14 -fsyntax-only -verify
+ /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/llvm/s390x-redhat-linux-gnu/bin/../lib/clang/20/include -nostdsysteminc /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp -triple x86_64-linux-gnu -fexperimental-new-constant-interpreter -std=c++14 -fsyntax-only -verify
error: 'expected-error' diagnostics seen but not expected: 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp Line 229: static assertion failed due to requirement 's[0] == 12': 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp Line 232: static assertion failed due to requirement 't[0] == 9': 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp Line 416: static assertion failed due to requirement 's[0] == 12': 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp Line 419: static assertion failed due to requirement 't[0] == 9': 
error: 'expected-note' diagnostics seen but not expected: 
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp Line 229: expression evaluates to ''<U+0006>' (0x06, 6) == 12'
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp Line 232: expression evaluates to ''<U+0013>' (0x13, 19) == 9'
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp Line 416: expression evaluates to ''<U+0006>' (0x06, 6) == 12'
  File /builddir/build/BUILD/llvm-project-00629752e622814649e67d6e5ecb02bf131b537d/clang/test/AST/ByteCode/constexpr-vectors.cpp Line 419: expression evaluates to ''<U+0013>' (0x13, 19) == 9'
8 errors generated.

--

********************

@yronglin
Copy link
Contributor Author

@yronglin Could you try to investigate?

Ok, I'll take a look.

@tbaederr
Copy link
Contributor

Ok, I managed to reproduce on a s390x host

@tbaederr
Copy link
Contributor

tbaederr commented Sep 24, 2024

For the code in question:

  using FourCharsVecSize __attribute__((vector_size(4))) = char;
  constexpr auto s = FourCharsVecSize{6, 3, 5, 10} << 1;

  static_assert(s[0] == 12 && s[1] == 6 && s[2] == 10 && s[3] == 20, "");

The AST is:

BinaryOperator 0x16251b8 'FourCharsVecSize':'__attribute__((__vector_size__(4 * sizeof(char)))) char' '<<'
|-CXXFunctionalCastExpr 0x1625120 'FourCharsVecSize':'__attribute__((__vector_size__(4 * sizeof(char)))) char' functional cast to FourCharsVecSize <NoOp>
| `-InitListExpr 0x1625060 'FourCharsVecSize':'__attribute__((__vector_size__(4 * sizeof(char)))) char'
|   |-ImplicitCastExpr 0x16250c0 'char' <IntegralCast>
|   | `-IntegerLiteral 0x1624f80 'int' 6
|   |-ImplicitCastExpr 0x16250d8 'char' <IntegralCast>
|   | `-IntegerLiteral 0x1624fa0 'int' 3
|   |-ImplicitCastExpr 0x16250f0 'char' <IntegralCast>
|   | `-IntegerLiteral 0x1624fc0 'int' 5
|   `-ImplicitCastExpr 0x1625108 'char' <IntegralCast>
|     `-IntegerLiteral 0x1624fe0 'int' 10
`-ImplicitCastExpr 0x16251a0 'int __attribute__((ext_vector_type(4)))' <VectorSplat>
  `-IntegerLiteral 0x1625148 'int' 1

so the assumption that the element types of RHS and LHS are the same does not hold.

This would've come up earlier if I had put an assertion in the right place:

diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 1f4c302b2619..d77646c2bdaf 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2584,6 +2584,7 @@ inline bool ArrayElemPop(InterpState &S, CodePtr OpPC, uint32_t Index) {
   if (!CheckLoad(S, OpPC, Ptr))
     return false;

+  assert(Ptr.atIndex(Index).getFieldDesc()->getPrimType() == Name);
   S.Stk.push<T>(Ptr.atIndex(Index).deref<T>());
   return true;
 }

tbaederr added a commit that referenced this pull request Sep 24, 2024
For shifts, the LHS and RHS element types might be different. The
variable naming here could probably use some love now, but I'm trying to
fix this as fast as possible.

See the discussion in #108949
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants