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

[Eli Friedman via Phabricator via llvm-commits]

efriedma 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;
+
----------------
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.


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

https://reviews.llvm.org/D76720