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

[Christopher Tetreault via Phabricator via lldb-commits]

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...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77587