[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

Christopher Tetreault via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 21 10:15:40 PDT 2020


ctetreau added a comment.

In D80323#2048457 <https://reviews.llvm.org/D80323#2048457>, @rjmccall wrote:

> I'm sympathetic to wanting to get rid of the boolean flag, but this is a really invasive change for pretty minimal benefit.  Why not leave `VectorType::get` as meaning a non-scalable vector type and come up with a different method name to get a scalable vector?


As it stands today, if you call VectorType::get(Ty, N, false), you will get an instance of FixedVectorType, via a base VectorType pointer. Currently, you can call getNumElements() on a base VectorType and it will work, but the ultimate endgame is that getNumElements is going to be removed from base VectorType. This change makes it so that you don't have to cast your VectorType object to FixedVectorType everywhere.

The overall architecture is that there is a derived type for each sort of vector. VectorType is the base class, and the two derived vector types are FixedVectorType and ScalableVectorType. I suppose I could have named them something like BaseVectorType, VectorType(actually fixed width vector type), and ScalableVectorType, but I feel that this is a worse solution because it's not obvious by the name what a "VectorType" is. Additionally, naming the types this would have broken all code that previously worked for scalable vectors. It's not obvious to me which naming scheme would have resulted in less changes (changes to rename fixed vectors to FixedVectorType vs changes needed to rename all generic code to BaseVectorType and to fix code for scalable vectors), but I think the consistency in the naming scheme justifies the path I chose.

For your specific proposal, I think it would be very strange if a function with the signature `static FixedVectorType *get(Type *, unsigned)` were a member of the base VectorType.

If you'd like to know more about the motivation for this work, here is a link to the RFC: http://lists.llvm.org/pipermail/llvm-dev/2020-March/139811.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323





More information about the cfe-commits mailing list