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

Anton Afanasyev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 11 08:13:10 PST 2019


anton-afanasyev marked 11 inline comments as done.
anton-afanasyev 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();
----------------
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.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6762
 static bool findBuildAggregate(InsertValueInst *IV,
-                               SmallVectorImpl<Value *> &BuildVectorOpds) {
+                             TargetTransformInfo *TTI,
+                             SmallVectorImpl<Value *> &BuildVectorOpds,
----------------
ABataev wrote:
> Not formatted
Thanks, fixed with `clang-format`.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6772-6774
+      for (auto OpdIt = TmpBuildVectorOpds.rbegin();
+           OpdIt != TmpBuildVectorOpds.rend(); OpdIt++)
+        BuildVectorOpds.push_back(*OpdIt);
----------------
ABataev wrote:
> Just `BuildVectorOpds.append(TmpBuildVectorOpds.rbegin(), TmpBuildVectorOpds.rend());`
Sure, thanks!


================
Comment at: llvm/test/Transforms/SLPVectorizer/X86/pr42022.ll:4
+
+%struct.Vector4 = type { float, float, float, float }
+
----------------
RKSimon wrote:
> ABataev wrote:
> > anton-afanasyev wrote:
> > > RKSimon wrote:
> > > > Regenerate with update_test_checks.py ?
> > > Should it really be autogenerated? I don't like test autogeneration for excessiveness and false positives while testing. Here we just checking vector `<4 x float>` is generated. Is autogeneration really standard for tests now?
> > It is common practice to use auto checks and demonstrate the difference then.
> update_test_checks.py helps us identify any hidden regressions
> 
> also you can probably remove the dso_local and local_unnamed_addr #0 tags
Ok, thank, I'm to fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70068





More information about the llvm-commits mailing list