[PATCH] D146872: [1/11][IR] Permit load/store/alloca for struct of the same scalable vector type

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/IR/DataLayout.cpp:49
+StructLayout::StructLayout(StructType *ST, const DataLayout &DL)
+    : StructSize(TypeSize::Fixed(0)) {
   assert(!ST->isOpaque() && "Cannot get layout of opaque structs");
----------------
eopXD wrote:
> craig.topper wrote:
> > Would StructSize be default constructed to TypeSize::Fixed(0) anyway?
> The ctors of `TypeSize` are:
> 
> ```
>   constexpr TypeSize(ScalarTy Quantity, bool Scalable)
>       : FixedOrScalableQuantity(Quantity, Scalable) {}
> 
>   static constexpr TypeSize getFixed(ScalarTy ExactSize) {
>     return TypeSize(ExactSize, false);
>   }
>   static constexpr TypeSize getScalable(ScalarTy MinimumSize) {
>     return TypeSize(MinimumSize, true);
>   }
>   static constexpr TypeSize get(ScalarTy Quantity, bool Scalable) {
>     return TypeSize(Quantity, Scalable);
>   }
>   static constexpr TypeSize Fixed(ScalarTy ExactSize) {
>     return TypeSize(ExactSize, false);
>   }
>   static constexpr TypeSize Scalable(ScalarTy MinimumSize) {
>     return TypeSize(MinimumSize, true);
>   }
> ```
> 
> Am I using it incorrectly here?
You're right. There is no default constructor. Though it's base class has one.


================
Comment at: llvm/lib/IR/Type.cpp:635
       return false;
+    if (StructType *STy = dyn_cast<StructType>(Ty);
+        STy && containsHomogeneousScalableVectorTypes())
----------------
Could we call isScalableTy instead of `isa<ScalableVectorType>(Ty)` above?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:767
 
+    if (ST->containsScalableVectorType())
+      return nullptr;
----------------
Does SL->getSizeInBits().isScalable() below make this unnecessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146872