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


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4474
   if (AI.isArrayAllocation() || !AI.getAllocatedType()->isSized() ||
-      DL.getTypeAllocSize(AI.getAllocatedType()) == 0)
+      AI.getAllocatedType()->isScalableVectorTy() ||
+      DL.getTypeAllocSize(AI.getAllocatedType()).getFixedSize() == 0)
----------------
This can be rewritten:

```
  {
    auto *AT = AI.getAllocatedType();
    if (AI.isArrayAllocation() || !AT->isSized() ||
        (isa<VectorType>(AT) && cast<VectorType>(AT)->isScalable()) ||
        DL.getTypeAllocSize(AT).getFixedSize() == 0)
      return false;
  }
```

AI.getAllocatedType is used 3 times, might as well give it a name. An while isa<VectorType>(AT) && cast<VectorType>(AT)->isScalable() is a little longer than AT.isScalableVectorTy, it's not that bad. on the positive side, it's more explicit as to what it's doing, and it's also less misleading because there's no such thing as a ScalableVectorTy.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4596
+    if (AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
+      if (AI->getAllocatedType()->isScalableVectorTy() &&
+          isAllocaPromotable(AI))
----------------
```
      if (isa<VectorType>(AI->getAllocatedType()) &&
          cast<VectorType>(AI->getAllocatedType())->isScalable() &&
          isAllocaPromotable(AI))
```

Same as above.


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

https://reviews.llvm.org/D76720