[PATCH] D70068: [SLP] Enhance SLPVectorizer to vectorize vector aggregate

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2879-2880
+  if (auto *VT = dyn_cast<VectorType>(EltTy)) {
+    if (VT->isScalable())
+      return 0;
+    EltTy = VT->getElementType();
----------------
anton-afanasyev wrote:
> ABataev wrote:
> > anton-afanasyev wrote:
> > > ABataev wrote:
> > > > ABataev wrote:
> > > > > Why do we have this check?
> > > > Also, would be good to have a test for this if we don't have it yet.
> > > Hmm, you're right, I'm just to delete this check. I haven't faced any issue concerning scalability, but decided to check it just in case (`{ <vscale x 2 x float>, <vscale x 2 x float> }` is not isomorphic to `<4 x float>`?). But I see no similar check across `SLPVectorizer.cpp`, so I'm to delete this check.
> > My question was different. I just wanted some explanation what's the problem with the scalable vectors? If the check is required we must have it. Also, still, would be good to have a test for this situation.
> I wasn't familiar with `vscale` property, but learned it now. That check was actually unnecessary: `{ <vscale x N x Ty>, <vscale x N x Ty> }` maps to `<vscale x 2N x Ty>`,  so we can deal with this scalability set to true.
Probably worth adding at least some test coverage for this - in aarch64 maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70068