[PATCH] D78601: [SVE] Do not store a bool for Scalable in VectorType

Francesco Petrogalli via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 24 05:54:17 PDT 2020


fpetrogalli added a comment.

Thank you for updating the patch @ctetreau

In D78601#1999638 <https://reviews.llvm.org/D78601#1999638>, @ctetreau wrote:

> [...] Maybe at some point in the future, a new vector type is added where the quantity is not a lower bound.


If that's the case, I think that your name is better. Otherwise we will have to find a better name if someone comes up with a different use of the field for some fancy vector type that don't treat it as a "mi num of element". Apologies for the extra work!

> I prefer not to add a public function to base VectorType that gets just an unsigned right now. It's certainly a reasonable function, but my concern is that right now we're in a transition period, and most people will want to take the path of least resistance to fix their code. I think we'll see a lot of code that looks like this:
> 
>   unsigned ActualNumElements = cast<VectorType>(Ty)->getNumElements();
>    
> 
> ... become:
> 
>   unsigned ActualNumElements = cast<VectorType>(Ty)->getMinNumElements();
>    
> 
> ... instead of something more robust.

Makes totally sense to me, thank you for explaining!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78601





More information about the llvm-commits mailing list