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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D145088#4259377 <https://reviews.llvm.org/D145088#4259377>, @craig.topper wrote:

> In D145088#4258856 <https://reviews.llvm.org/D145088#4258856>, @erichkeane wrote:
>
>> So I don't see any handling of the dependent version of this, we probably need tests for those at minimum.
>
> Does SVE handle the dependent version?

It does, I believe we insisted on it at the time.  You may inherit it sufficiently, so tests for it are perhaps all that is necessary.



================
Comment at: clang/include/clang/AST/ASTContext.h:2262
+  /// Return true if the given vector types are lax-compatible RISC-V vector
+  /// types as defined by -flax-vector-conversions=, false otherwise.
+  bool areLaxCompatibleRVVTypes(QualType FirstType, QualType SecondType);
----------------
I still had to look this one up.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:11371
+  unsigned EltSize = getContext().getTypeSize(BT);
+  switch (BT->getKind()) {
+  default:
----------------
Having the switch still is awkward, since it only exists for an unreachable.  I wonder if splitting off this type checking to a separate function and asserting on it is more valuable?  AND could be used elsewhere if we use this pattern again?

I'll leave that up to the CodeGen code owners to require however.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145088