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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to