[PATCH] D76720: [Transforms][SROA] Promote allocas with mem2reg for scalable types

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added inline comments.


================
Comment at: llvm/include/llvm/IR/Type.h:233
+  /// True if this an instance of VectorType with the scalable property set.
+  bool isScalableVectorTy() const;
+
----------------
efriedma wrote:
> c-rhodes wrote:
> > ctetreau wrote:
> > > Personally, I think functions like this are a code smell. Why have a type hierarchy at all if we're just going to do everything through the base type?
> > > 
> > > However, given the fact that the VectorType hierarchy is going to become more complicated soon, I'd really prefer that this function not be added. I'll propose alternatives inline below, but if you want this function, I'd prefer that:
> > > 
> > > 1) be a static function in SROA.cpp.
> > > 2) be implemented in terms of isa<VectorType> instead of isVectorTy.
> > Thanks for the suggestion, I've removed this. I don't think it's particularly useful as a function in SROA given it isn't (and is unlikely) to be used a great deal.
> I think for code that specifically cares about the property "would getTypeAllocSize() return a scalable TypeSize", it's not unreasonable to have a function on Type to query that.  The key here being that the code doesn't really care whether the type is a vector.
> 
> It might make sense to leave it out for now, though, and try to refactor later when we have a better idea what code ends up doing in practice.  I think most code that would want to do that calls getTypeAllocSize() somewhere nearby anyway.
> Thanks for the suggestion, I've removed this. I don't think it's particularly useful as a function in SROA given it isn't (and is unlikely) to be used a great deal.

Appreciate it.

> I think for code that specifically cares about the property "would getTypeAllocSize() return a scalable TypeSize", it's not unreasonable to have a function on Type to query that. The key here being that the code doesn't really care whether the type is a vector.

I'd feel better about this function, but I'd like to see it become a more common issue before we add it; Type is cluttered enough as it is.


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

https://reviews.llvm.org/D76720