[clang] Allow vector of BitInt

[python3kgae]

Summary

Remove check which disable BitInt as element type for ext_vector.

Enabling it for HLSL to use _BitInt(16) as 16bit int at https://reviews.llvm.org/D133668

[python3kgae]

Update test.

[xbolva00]

Missing IR tests. No issues in codegen?

Very poor description. Why it was disabled? Why we can enable it now?

[python3kgae]

In D133634#3782999, @xbolva00 wrote:

Missing IR tests. No issues in codegen?

Very poor description. Why it was disabled? Why we can enable it now?

I'm not sure why it is disabled.
I guess it is never enabled and just assert in ASTContext::getExtVectorType before.
Then https://github.com/llvm/llvm-project/commit/c9edf843fcf954132271214445857498fb47bb72 make it an error instead of assert.

I'm enabling it for HLSL to use _BitInt(16) as 16bit int at https://reviews.llvm.org/D133668
This PR only makes sure it does not fail at AST level.
https://reviews.llvm.org/D133668 will fix the fail in Mangling when codeGen.

[aaron.ballman]

In D133634#3783160, @python3kgae wrote:

In D133634#3782999, @xbolva00 wrote:

Missing IR tests. No issues in codegen?

Very poor description. Why it was disabled? Why we can enable it now?

I'm not sure why it is disabled.
I guess it is never enabled and just assert in ASTContext::getExtVectorType before.
Then https://github.com/llvm/llvm-project/commit/c9edf843fcf954132271214445857498fb47bb72 make it an error instead of assert.

It was disabled because it wasn't intentionally enabled -- we've not thought through the best way to expose vectors of _BitInt yet and we didn't want anyone to get tied into an accidental ABI by leaving them exposed.

I'm enabling it for HLSL to use _BitInt(16) as 16bit int at https://reviews.llvm.org/D133668
This PR only makes sure it does not fail at AST level.
https://reviews.llvm.org/D133668 will fix the fail in Mangling when codeGen.

I think this requires more consideration. I think we *want* to enable vectors of _BitInt types, but I think we need a more full-featured plan for supporting them. For example, do we want to allow a vector of _BitInt(5) objects, or do we want to support only powers-of-two sized _BitInts? How do we ensure ABI compatibility with GCC for these vector types? That sort of thing.

CC @erichkeane who worked on the initial _ExtInt implementation in case he's got additional thoughts.

[erichkeane]

There was no motivation for disabling this other than not wanting/caring enough to define the ABI and writing sufficient tests. While I don't think there is sufficient testing here around ABI/etc, I have no problem with this.

[craig.topper]

I'm a little nervous about non-byte-sized non-power of 2 elements. The memory layout handling gets a little weird in the backend. @efriedma what do you think?

[efriedma]

I think the worst of the backend issues with vectors of non-byte-size integers have been resolved? Work on that has progressed slowly for a very long time. There have been a constant stream of issues. (See, for example, D129164.) But LangRef does define the memory layout, and CodeGen does essentially implement that layout.

It's not clear to me we actually want to use that layout for vectors of non-power-of-two types. Load/store for bit-packed vectors isn't efficient.

Maybe we can just enable this specifically for types which are a power of two multiple of 8 for now (where there's one obvious layout), and consider the other cases later?

[python3kgae]

Limit BitInt vector to byte-sized and power of 2 BitInt.

[python3kgae]

Thanks for all the comments.
I've limit BitInt vector to BitInt with byte-sized and power of 2 NumBits.

For the ABI, is it OK to let mangleType of VectorType treat BitInt like other element types, call mangleType of BitInt for the element type part?

[erichkeane]

The CFE Changes look correct, but @eli.friedman/@craig.topper should comment as to whether this is acceptable.

It still needs a few codegen tests however, to make sure the basic operations work, and that these are passed correctly.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3024

We might as well spell out the failure conditions and make a more readable diagnostic.

[python3kgae]

Allow BitInt vector for Microsoftmangle and add codeGen tests for BitInt vector.

[craig.topper]

In D133634#3795044, @erichkeane wrote:

The CFE Changes look correct, but @eli.friedman/@craig.topper should comment as to whether this is acceptable.

This is acceptable to me.

clang/lib/AST/MicrosoftMangle.cpp

3111 ↗

(On Diff #460958)

Why is an explicit cast needed here?

[python3kgae]

clang/lib/AST/MicrosoftMangle.cpp

3111 ↗

(On Diff #460958)

Because ET and BitIntTy have incompatible types for the select. (error : incompatible operand types ('const clang::BuiltinType *' and 'const clang::BitIntType *')

cast to const Type * will make them agree as const Type *.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_attribute_parameter_types : Error<
   "%0 attribute parameter types do not match: parameter %1 of function %2 has type %3, "
   "but parameter %4 of function %5 has type %6">;
 
 def err_attribute_too_many_arguments : Error<
   "%0 attribute takes no more than %1 argument%s1">;
 def err_attribute_too_few_arguments : Error<
   "%0 attribute takes at least %1 argument%s1">;
 def err_attribute_invalid_vector_type : Error<"invalid vector element type %0">;
+def err_attribute_invalid_bitint_vector_type : Error<"invalid vector element type %0, must be byte-sized and power of 2 _BitInt">;
 def err_attribute_invalid_matrix_type : Error<"invalid matrix element type %0">;
 def err_attribute_bad_neon_vector_size : Error<
   "Neon vector size must be 64 or 128 bits">;
 def err_attribute_invalid_sve_type : Error<
   "%0 attribute applied to non-SVE type %1">;
 def err_attribute_bad_sve_vector_size : Error<
   "invalid SVE vector size '%0', must match value set by "
   "'-msve-vector-bits' ('%1')">;

clang/lib/AST/ASTContext.cpp

#include "clang/Basic/RISCVVTypes.def"
   }
   return QualType();
 }
 
 /// getVectorType - Return the unique reference to a vector type of
 /// the specified element type and size. VectorType must be a built-in type.
 QualType ASTContext::getVectorType(QualType vecType, unsigned NumElts,
                                    VectorType::VectorKind VecKind) const {
-  assert(vecType->isBuiltinType());
+  assert(vecType->isBuiltinType() ||
+         (vecType->isBitIntType() &&
+          // Only support _BitInt elements with byte-sized power of 2 NumBits.
+          llvm::isPowerOf2_32(vecType->getAs<BitIntType>()->getNumBits()) &&
+          vecType->getAs<BitIntType>()->getNumBits() >= 8));
 
   // Check if we've already instantiated a vector of this type.
   llvm::FoldingSetNodeID ID;
   VectorType::Profile(ID, vecType, NumElts, Type::Vector, VecKind);
 
   void *InsertPos = nullptr;
   if (VectorType *VTP = VectorTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(VTP, 0);
ASTContext::getDependentVectorType(QualType VecType, Expr *SizeExpr,
   }
 
   Types.push_back(New);
   return QualType(New, 0);
 }
 
 /// getExtVectorType - Return the unique reference to an extended vector type of
 /// the specified element type and size. VectorType must be a built-in type.
-QualType
-ASTContext::getExtVectorType(QualType vecType, unsigned NumElts) const {
-  assert(vecType->isBuiltinType() || vecType->isDependentType());
+QualType ASTContext::getExtVectorType(QualType vecType,
+                                      unsigned NumElts) const {
+  assert(vecType->isBuiltinType() || vecType->isDependentType() ||
+         (vecType->isBitIntType() &&
+          // Only support _BitInt elements with byte-sized power of 2 NumBits.
+          llvm::isPowerOf2_32(vecType->getAs<BitIntType>()->getNumBits()) &&
+          vecType->getAs<BitIntType>()->getNumBits() >= 8));
 
   // Check if we've already instantiated a vector of this type.
   llvm::FoldingSetNodeID ID;
   VectorType::Profile(ID, vecType, NumElts, Type::ExtVector,
                       VectorType::GenericVector);
   void *InsertPos = nullptr;
   if (VectorType *VTP = VectorTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(VTP, 0);

clang/lib/Sema/SemaType.cpp

 }
 
 QualType Sema::BuildVectorType(QualType CurType, Expr *SizeExpr,
                                SourceLocation AttrLoc) {
   // The base type must be integer (not Boolean or enumeration) or float, and
   // can't already be a vector.
   if ((!CurType->isDependentType() &&
        (!CurType->isBuiltinType() || CurType->isBooleanType() ||
-        (!CurType->isIntegerType() && !CurType->isRealFloatingType()))) ||
+        (!CurType->isIntegerType() && !CurType->isRealFloatingType())) &&
+       !CurType->isBitIntType()) ||
       CurType->isArrayType()) {
     Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << CurType;
     return QualType();
   }
+  // Only support _BitInt elements with byte-sized power of 2 NumBits.
+  if (CurType->isBitIntType()) {
+    unsigned NumBits = CurType->getAs<BitIntType>()->getNumBits();
+    if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
+      Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type) << CurType;
+      return QualType();
+    }
+  }
 
   if (SizeExpr->isTypeDependent() || SizeExpr->isValueDependent())
     return Context.getDependentVectorType(CurType, SizeExpr, AttrLoc,
                                                VectorType::GenericVector);
 
   Optional<llvm::APSInt> VecSize = SizeExpr->getIntegerConstantExpr(Context);
   if (!VecSize) {
     Diag(AttrLoc, diag::err_attribute_argument_type)
QualType Sema::BuildExtVectorType(QualType T, Expr *ArraySize,
   // Additionally, OpenCL prohibits vectors of booleans (they're considered a
   // reserved data type under OpenCL v2.0 s6.1.4), we don't support selects
   // on bitvectors, and we have no well-defined ABI for bitvectors, so vectors
   // of bool aren't allowed.
   //
   // We explictly allow bool elements in ext_vector_type for C/C++.
   bool IsNoBoolVecLang = getLangOpts().OpenCL || getLangOpts().OpenCLCPlusPlus;
   if ((!T->isDependentType() && !T->isIntegerType() &&
-       !T->isRealFloatingType()) || T->isBitIntType() ||
+       !T->isRealFloatingType()) ||
       (IsNoBoolVecLang && T->isBooleanType())) {
     Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << T;
     return QualType();
   }
 
+  // Only support _BitInt elements with byte-sized power of 2 NumBits.
+  if (T->isBitIntType()) {
+    unsigned NumBits = T->getAs<BitIntType>()->getNumBits();
+    if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
+      Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type) << T;
+      return QualType();
+    }
+  }
+
   if (!ArraySize->isTypeDependent() && !ArraySize->isValueDependent()) {
     Optional<llvm::APSInt> vecSize = ArraySize->getIntegerConstantExpr(Context);
     if (!vecSize) {
       Diag(AttrLoc, diag::err_attribute_argument_type)
         << "ext_vector_type" << AANT_ArgumentIntegerConstant
         << ArraySize->getSourceRange();
       return QualType();
     }

clang/test/Sema/builtin-classify-type.c

void foo(void) {
   extern void f(void);
   struct { int a; float b; } s_obj;
   union { int a; float b; } u_obj;
   int arr[10];
   int (^block)(void);
   __attribute__((vector_size(16))) int vec;
   typedef __attribute__((ext_vector_type(4))) int evec_t;
   evec_t evec;
+  typedef _BitInt(8) int8_t3 __attribute__((ext_vector_type(3)));
+  int8_t3 t3;
+  typedef _BitInt(16) int16_t3 __attribute__((ext_vector_type(4)));
+  int16_t3 t4;
+  typedef _BitInt(32) int32_t3 __attribute__((ext_vector_type(5)));
+  int32_t3 t5;
+  typedef _BitInt(64) int64_t3 __attribute__((ext_vector_type(6)));
+  int64_t3 t6;
+  typedef _BitInt(8) vint8_t3 __attribute__((vector_size(3)));
+  vint8_t3 vt3;
+  typedef _BitInt(16) vint16_t3 __attribute__((vector_size(4)));
+  vint16_t3 vt4;
+  typedef _BitInt(32) vint32_t3 __attribute__((vector_size(8)));
+  vint32_t3 vt5;
+  typedef _BitInt(64) vint64_t3 __attribute__((vector_size(16)));
+  vint64_t3 vt6;
+
   _Atomic int atomic_i;
   _Atomic double atomic_d;
   _Complex int complex_i;
   _Complex double complex_d;
 
   int a1[__builtin_classify_type(f()) == void_type_class ? 1 : -1];
   int a2[__builtin_classify_type(i) == integer_type_class ? 1 : -1];
   int a3[__builtin_classify_type(c) == integer_type_class ? 1 : -1];

clang/test/SemaCXX/ext-int.cpp

struct is_same {
   static constexpr bool value = false;
 };
 template <typename T>
 struct is_same<T,T> {
   static constexpr bool value = true;
 };
 
 // Reject vector types:
-// expected-error@+1{{invalid vector element type '_BitInt(32)'}}
-typedef _BitInt(32) __attribute__((vector_size(16))) VecTy;
-// expected-error@+1{{invalid vector element type '_BitInt(32)'}}
-typedef _BitInt(32) __attribute__((ext_vector_type(32))) OtherVecTy;
+// expected-error@+1{{invalid vector element type '_BitInt(2)', must be byte-sized and power of 2 _BitInt}}
+typedef _BitInt(2) __attribute__((vector_size(16))) VecTy;
+// expected-error@+1{{invalid vector element type '_BitInt(2)', must be byte-sized and power of 2 _BitInt}}
+typedef _BitInt(2) __attribute__((ext_vector_type(32))) OtherVecTy;
+// expected-error@+1{{invalid vector element type '_BitInt(4)', must be byte-sized and power of 2 _BitInt}}
+typedef _BitInt(4) __attribute__((vector_size(16))) VecTy2;
+// expected-error@+1{{invalid vector element type '_BitInt(4)', must be byte-sized and power of 2 _BitInt}}
+typedef _BitInt(4) __attribute__((ext_vector_type(32))) OtherVecTy2;
+// expected-error@+1{{invalid vector element type '_BitInt(5)', must be byte-sized and power of 2 _BitInt}}
+typedef _BitInt(5) __attribute__((vector_size(16))) VecTy3;
+// expected-error@+1{{invalid vector element type '_BitInt(5)', must be byte-sized and power of 2 _BitInt}}
+typedef _BitInt(5) __attribute__((ext_vector_type(32))) OtherVecTy3;
+// expected-error@+1{{invalid vector element type '_BitInt(37)', must be byte-sized and power of 2 _BitInt}}
+typedef _BitInt(37) __attribute__((vector_size(16))) VecTy4;
+// expected-error@+1{{invalid vector element type '_BitInt(37)', must be byte-sized and power of 2 _BitInt}}
+typedef _BitInt(37) __attribute__((ext_vector_type(32))) OtherVecTy4;
 
 // Allow _Complex:
 _Complex _BitInt(3) Cmplx;
 
 // Reject cases of _Atomic:
 // expected-error@+1{{_Atomic cannot be applied to integer type '_BitInt(4)'}}
 _Atomic _BitInt(4) TooSmallAtomic;
 // expected-error@+1{{_Atomic cannot be applied to integer type '_BitInt(9)'}}