[riscv] Fix for __riscv_v_fixed_vlen in vector mask types

[ita-sc]

It is already possible to have vector types with riscv_rvv_vector_bits attribute as structure members. But currently this is not the case for vector masks. Having vector masks in structures is useful for library implementations.

This patch removes restriction for using RISC-V vector mask types in structures.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-clang

Author: None (ita-sc)

Changes

It is already possible to have vector types with riscv_rvv_vector_bits attribute as structure members. But currently this is not the case for vector masks. Having vector masks in structures is useful for library implementations.

This patch removes restriction for using RISC-V vector mask types in structures.

---

Full diff: https://github.com/llvm/llvm-project/pull/76510.diff

5 Files Affected:

• (modified) clang/include/clang/Basic/AttrDocs.td (+2-1)
• (modified) clang/lib/AST/ItaniumMangle.cpp (+3)
• (modified) clang/lib/AST/Type.cpp (+4)
• (modified) clang/lib/Sema/SemaType.cpp (+3)
• (modified) clang/test/Sema/attr-riscv-rvv-vector-bits.c (+28-3)

diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 98a7ecc7fd7df3..d50a57e7e0c7e8 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2415,7 +2415,8 @@ only be a power of 2 between 64 and 65536.
 For types where LMUL!=1, ``__riscv_v_fixed_vlen`` needs to be scaled by the LMUL
 of the type before passing to the attribute.
 
-``vbool*_t`` types are not supported at this time.
+For ``vbool*_t`` types, ``__riscv_v_fixed_vlen`` needs to be divided by the EEW/LMUL
+(e.g. for vbool64_t we need ``N==(__riscv_v_fixed_vlen/64)``).
 }];
 }
 
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b1678479888eb7..b2e9ffd4231ead 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -4005,6 +4005,9 @@ void CXXNameMangler::mangleRISCVFixedRVVVectorType(const VectorType *T) {
   llvm::raw_svector_ostream TypeNameOS(TypeNameStr);
   TypeNameOS << "__rvv_";
   switch (cast<BuiltinType>(EltType)->getKind()) {
+  case BuiltinType::Bool:
+    TypeNameOS << "int1";
+    break;
   case BuiltinType::SChar:
     TypeNameOS << "int8";
     break;
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 160a725939ccd4..4a80fe83dfe8a2 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2479,6 +2479,10 @@ bool Type::isRVVVLSBuiltinType() const {
                         IsFP, IsBF)                                            \
   case BuiltinType::Id:                                                        \
     return NF == 1;
+
+#define RVV_PREDICATE_TYPE(Name, Id, SingletonId, NumEls)                      \
+  case BuiltinType::Id:                                                        \
+    return true;
 #include "clang/Basic/RISCVVTypes.def"
     default:
       return false;
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index a376f20fa4f4e0..740564a17aa6cb 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8543,8 +8543,11 @@ static void HandleRISCVRVVVectorBitsTypeAttr(QualType &CurType,
   ASTContext::BuiltinVectorTypeInfo Info =
       S.Context.getBuiltinVectorTypeInfo(CurType->castAs<BuiltinType>());
   unsigned EltSize = S.Context.getTypeSize(Info.ElementType);
+  if (Info.ElementType->isBooleanType())
+    EltSize = 1;
   unsigned MinElts = Info.EC.getKnownMinValue();
 
+
   // The attribute vector size must match -mrvv-vector-bits.
   unsigned ExpectedSize = VScale->first * MinElts * EltSize;
   if (VecSize != ExpectedSize) {
diff --git a/clang/test/Sema/attr-riscv-rvv-vector-bits.c b/clang/test/Sema/attr-riscv-rvv-vector-bits.c
index fe507a102cee1e..6fc5954b01035b 100644
--- a/clang/test/Sema/attr-riscv-rvv-vector-bits.c
+++ b/clang/test/Sema/attr-riscv-rvv-vector-bits.c
@@ -220,6 +220,15 @@ typedef uint64_t gnu_uint64m8_t __attribute__((vector_size(__riscv_v_fixed_vlen)
 typedef float gnu_float32m8_t __attribute__((vector_size(__riscv_v_fixed_vlen)));
 typedef double gnu_float64m8_t __attribute__((vector_size(__riscv_v_fixed_vlen)));
 
+
+typedef vbool1_t fixed_vbool1_t_t __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen)));
+typedef vbool2_t fixed_vbool2_t_t __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen / 2)));
+typedef vbool4_t fixed_vbool4_t_t __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen / 4)));
+typedef vbool8_t fixed_vbool8_t_t __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen / 8)));
+typedef vbool16_t fixed_vbool16_t_t __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen / 16)));
+typedef vbool32_t fixed_vbool32_t_t __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen / 32)));
+typedef vbool64_t fixed_vbool64_t_t __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen / 64)));
+
 // Attribute must have a single argument
 typedef vint8m1_t no_argument __attribute__((riscv_rvv_vector_bits));         // expected-error {{'riscv_rvv_vector_bits' attribute takes one argument}}
 typedef vint8m1_t two_arguments __attribute__((riscv_rvv_vector_bits(2, 4))); // expected-error {{'riscv_rvv_vector_bits' attribute takes one argument}}
@@ -228,9 +237,6 @@ typedef vint8m1_t two_arguments __attribute__((riscv_rvv_vector_bits(2, 4))); //
 typedef vint8m1_t non_int_size1 __attribute__((riscv_rvv_vector_bits(2.0)));   // expected-error {{'riscv_rvv_vector_bits' attribute requires an integer constant}}
 typedef vint8m1_t non_int_size2 __attribute__((riscv_rvv_vector_bits("256"))); // expected-error {{'riscv_rvv_vector_bits' attribute requires an integer constant}}
 
-// bool types and LMUL != 1 are not supported.
-typedef vbool1_t fixed_vbool1_t_t __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen))); // expected-error {{'riscv_rvv_vector_bits' attribute applied to non-RVV type 'vbool1_t'}}
-
 // Attribute must be attached to a single RVV vector or predicate type.
 typedef void *badtype1 __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen)));         // expected-error {{'riscv_rvv_vector_bits' attribute applied to non-RVV type 'void *'}}
 typedef int badtype2 __attribute__((riscv_rvv_vector_bits(__riscv_v_fixed_vlen)));           // expected-error {{'riscv_rvv_vector_bits' attribute applied to non-RVV type 'int'}}
@@ -398,6 +404,14 @@ _Static_assert(sizeof(fixed_int64m8_t) == VECTOR_SIZE * 8, "");
 _Static_assert(sizeof(fixed_float32m8_t) == VECTOR_SIZE * 8, "");
 _Static_assert(sizeof(fixed_float64m8_t) == VECTOR_SIZE * 8, "");
 
+_Static_assert(sizeof(fixed_vbool1_t_t) == VECTOR_SIZE * 8, "");
+_Static_assert(sizeof(fixed_vbool2_t_t) == VECTOR_SIZE * 8 / 2, "");
+_Static_assert(sizeof(fixed_vbool4_t_t) == VECTOR_SIZE * 8 / 4, "");
+_Static_assert(sizeof(fixed_vbool8_t_t) == VECTOR_SIZE  * 8 / 8, "");
+_Static_assert(sizeof(fixed_vbool16_t_t) == VECTOR_SIZE * 8 / 16, "");
+_Static_assert(sizeof(fixed_vbool32_t_t) == VECTOR_SIZE * 8 / 32, "");
+_Static_assert(sizeof(fixed_vbool64_t_t) == VECTOR_SIZE * 8 / 64, "");
+
 // --------------------------------------------------------------------------//
 // Alignof
 
@@ -475,9 +489,20 @@ _Static_assert(__alignof__(fixed_uint64m8_t) == VECTOR_ALIGN, "");
 _Static_assert(__alignof__(fixed_float32m8_t) == VECTOR_ALIGN, "");
 _Static_assert(__alignof__(fixed_float64m8_t) == VECTOR_ALIGN, "");
 
+
+_Static_assert(__alignof__(fixed_vbool1_t_t) == (sizeof(fixed_vbool1_t_t) < VECTOR_ALIGN ? sizeof(fixed_vbool1_t_t) : VECTOR_ALIGN), "");
+_Static_assert(__alignof__(fixed_vbool2_t_t) == (sizeof(fixed_vbool2_t_t) < VECTOR_ALIGN ? sizeof(fixed_vbool2_t_t) : VECTOR_ALIGN), "");
+_Static_assert(__alignof__(fixed_vbool4_t_t) == (sizeof(fixed_vbool4_t_t) < VECTOR_ALIGN ? sizeof(fixed_vbool4_t_t) : VECTOR_ALIGN), "");
+_Static_assert(__alignof__(fixed_vbool8_t_t) == (sizeof(fixed_vbool8_t_t) < VECTOR_ALIGN ? sizeof(fixed_vbool8_t_t) : VECTOR_ALIGN), "");
+_Static_assert(__alignof__(fixed_vbool16_t_t) == (sizeof(fixed_vbool16_t_t) < VECTOR_ALIGN ? sizeof(fixed_vbool16_t_t) : VECTOR_ALIGN), "");
+_Static_assert(__alignof__(fixed_vbool32_t_t) == (sizeof(fixed_vbool32_t_t) < VECTOR_ALIGN ? sizeof(fixed_vbool32_t_t) : VECTOR_ALIGN), "");
+_Static_assert(__alignof__(fixed_vbool64_t_t) == (sizeof(fixed_vbool64_t_t) < VECTOR_ALIGN ? sizeof(fixed_vbool64_t_t) : VECTOR_ALIGN), "");
+
 // --------------------------------------------------------------------------//
 // Structs
 
+struct struct_vbool4 {fixed_vbool4_t_t x, y[5];};
+
 struct struct_int64 { fixed_int64m1_t x, y[5]; };
 struct struct_float64 { fixed_float64m1_t x, y[5]; };
 

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff ae0b2633c935950084860e5f6a1c2c3203726489 69bfbb84879b41d4e8e5fbbd51b9b95b0767c460 -- clang/lib/AST/ItaniumMangle.cpp clang/lib/AST/Type.cpp clang/lib/Sema/SemaType.cpp clang/test/Sema/attr-riscv-rvv-vector-bits.c

View the diff from clang-format here.

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 740564a17a..39ac3701a5 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8547,7 +8547,6 @@ static void HandleRISCVRVVVectorBitsTypeAttr(QualType &CurType,
     EltSize = 1;
   unsigned MinElts = Info.EC.getKnownMinValue();
 
-
   // The attribute vector size must match -mrvv-vector-bits.
   unsigned ExpectedSize = VScale->first * MinElts * EltSize;
   if (VecSize != ExpectedSize) {

[topperc]

Missing CodeGen tests

[topperc]

I also have a patch for this that I started on 6 months ago. Let me dig it up and see how far along it was. I recall there being some tricky issues.