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

[Yueh-Ting (eop) Chen via Phabricator via llvm-commits]

eopXD added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:160
+              (isa<StructType>(Ty) &&
+               cast<StructType>(Ty)->containsScalableVectorType())) {
             MF->getFrameInfo().setStackID(FrameIndex,
----------------
zixuan-wu wrote:
> eopXD wrote:
> > nikic wrote:
> > > Use `isScalableTy()`?
> > Changing this to checking `Ty->isScalableTy()` results in changes the code generation of `llvm/test/CodeGen/AArch64/sme-aarch64-svcount.ll`.  The change makes sense to me, but the code generation is worse here, @sdesmalen may you confirm if this is reasonable?
> > Changing this to checking `Ty->isScalableTy()` results in changes the code generation of `llvm/test/CodeGen/AArch64/sme-aarch64-svcount.ll`.  The change makes sense to me, but the code generation is worse here, @sdesmalen may you confirm if this is reasonable?
> 
> Hi. As Ty->isScalableTy() contains target scalable type, it extends the semantic. So it's not only for scalable vector type. I think we can pass the ty as argument to distinguish them in target TFI, then getStackIDForScalableVectors would be better to be named 'getStackIDForScalableTy'
Thank you for checking this out. I will come back to revisit this when I finished implementing the other RVV intrinsic features for v1.0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146872