[Lldb-commits] [PATCH] D77587: [SVE] Add new VectorType subclasses

Christopher Tetreault via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 14 09:38:51 PDT 2020

ctetreau marked an inline comment as done.
ctetreau added inline comments.

Comment at: llvm/include/llvm/IR/DerivedTypes.h:430
+  /// Construct a VectorType that has the same shape as some other VectorType
+  static VectorType *get(Type *ElementType, VectorType *Other) {
+    return VectorType::get(ElementType, Other->getElementCount());
ctetreau wrote:
> efriedma wrote:
> > It seems confusing to overload get() this way.
> There's a bunch of code that is "I want a vector with the same shape as some other vector, but with a different element type"
> Currently, there's a bunch of variants like:
> ```
> auto *V2 = VectorType::Get(SomeTy, V1->getNumElements());
> auto *V3 = VectorType::Get(SomeTy, V4->getElementType());
> ```
> Really, for all variants of this operation, the first is potentially buggy if v1 is scalable, and it can always be replaced with the second and be correct (unless you are specifically trying to get a fixed width vector that has the same minimum number of elements as some potentially scalable vector, but that's a strange special case and I'd ask in code review that the author specifically pass false). But really, the quantity that you get out of the second argument is always noise. This change makes the following equivalent:
> ```
> auto *V2 = VectorType::Get(SomeTy, V1->getElementType());
> auto *V3 = VectorType::Get(SomeTy, V1);
> assert(V2->getType() == V3->getType());
> ```
> If V1 was scalable, then V2 is scalable, and vice versa. If you like, I can improve the documentation for this function, but I think it adds value.
In this example, "getElementType" should be "getElementCount", but I assume you get the idea...

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list