[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 ↗

(On Diff #460659)

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/lib/AST/ASTContext.cpp

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());
 
   // 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::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();
   }
 
   if (!ArraySize->isTypeDependent() && !ArraySize->isValueDependent()) {
     Optional<llvm::APSInt> vecSize = ArraySize->getIntegerConstantExpr(Context);
     if (!vecSize) {

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(16) int16_t3 __attribute__((ext_vector_type(3)));
+  int16_t3 t3;
   _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];