[PATCH] D82419: [SVE] add derived vector get(Type *, ElementCount) and get(Type *, VectorType)

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added a comment.

In D82419#2110388 <https://reviews.llvm.org/D82419#2110388>, @efriedma wrote:

> > Prior to this change, if a developer were to write:
> > 
> > auto *VTy = FixedVectorType::get(SomeTy, SomeVTy->getElementCount())
>
> This is just a compile error on master.  (You could write "using VectorType::get" in the class to force overloading, but definitions in derived classes hide definitions from the base class by default.)
>
>  -----
>
> Could you give an example where you plan to use this?  I don't expect it's common for code to take an arbitrary vector type, and try to produce a specific vector type based on it.


This case came up in a code review comment on D82218 <https://reviews.llvm.org/D82218>. I had made a change like `return FixedVectorType::get(Ty, cast<FixedVectorType>(VTy)->getNumElements());` and the reviewer asked why I was casting when I could just do `return FixedVectorType::get(Ty, VTy->getElementCount());`. My assumption was that it would just resolve to `VectorType::get(Ty, VTy->getElementCount())`. My concern was that one could call `FixedVectorType::get` and get an instance of `ScalableVectorType`. While this is what would have happened if get were a free function, clearly this is not what happens with it being a static function.

There does still exist a valid use case for these new functions. It can simplify code like:

  FixedVectorType *NewFVTy = nullptr;
  if (auto *FVTy = dyn_cast<FixedVectorType>(VTy)) {
     NewFVTy = FixedVectorType::get(SomeTy, FVTy);
  }

... into something like:

  FixedVectorType *NewFVTy = FixedVectorType::get(SomeTy, VTy)

My main concern was the perceived type mismatch due to overload resolution. However, since that can't happen, I'm not 100% sure these new functions carry their own weight.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82419