[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:2341
+``N==LMUL*__RISCV_RVV_VLEN_BITS``, the implementation defined feature macro that
+is enabled under the ``-mrvv-vector-bits`` flag.
+}];
----------------
You should add some details about requirements on the argument to the attribute (like the range of valid values, that it needs to be a power-of-two value, etc) and what happens when you write the attribute on a non-sizeless type.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3058
+  "%0 is only supported when '-mrvv-vector-bits=<bits>' is specified with a "
+  "value of \"zvl\" or a power 2 in the range [64,65536].">;
+def err_attribute_bad_rvv_vector_size : Error<
----------------



================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3897-3899
+  } else if (T->getVectorKind() == VectorType::RVVFixedLengthDataVector) {
+    mangleRISCVFixedRVVVectorType(T);
+    return;
----------------
Should there be corresponding changes to the Microsoft mangler as well?


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3926-3928
+  } else if (T->getVectorKind() == VectorType::RVVFixedLengthDataVector) {
+    mangleRISCVFixedRVVVectorType(T);
+    return;
----------------
Same here.


================
Comment at: clang/lib/AST/TypePrinter.cpp:703-706
     OS << " * sizeof(";
     print(T->getElementType(), OS, StringRef());
     // Multiply by 8 for the number of bits.
     OS << ") * 8))) ";
----------------
Bummer we don't have an `ASTContext` handy so we could call `getTypeSizeInChars()`... 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145088/new/

https://reviews.llvm.org/D145088