[PATCH] D105020: [SLP]Improve graph reordering.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:592
+  assert(!Mask.empty() && "Expected non-empty mask.");
+  SmallVector<Value *> Prev(Scalars.size());
+  Prev.swap(Scalars);
----------------
RKSimon wrote:
> Do we need a default value for vector?
Not currently, but I'll update these functions to make them compatible with upcoming non-power-2 vectorization.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2543
+  bool IsIdentity = true;
+  SmallVector<unsigned> Prev(Order.size());
+  Prev.swap(Order);
----------------
RKSimon wrote:
> Do we need a default value for vector?
No, it is swapped with `Order` and `Order` then updated. But I'll add default initialization because we may need it in the future for non-pow-2 patch.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2547
+    Order[Mask[Prev[I]]] = I;
+    IsIdentity &= I == Order[I];
+  }
----------------
RKSimon wrote:
> Doesn't the IsIdentity pass have to be done after Order[] is updated?
Yes, you're right, will fix this. It may affect the cost in some rare cases.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2559
+         "Expected non-empty mask and reuses mask.");
+  SmallVector<int> Prev(Reuses.size());
+  Prev.swap(Reuses);
----------------
RKSimon wrote:
> Do we need a default value for vector?
I'll add `UndefMaskElem` to reduce future updates for non-power-2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105020