[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:4188
+      const unsigned Sz = DL->getTypeSizeInBits(E->getMainOp()->getType());
+      for (unsigned MinVF = getMinVecRegSize() / (2 * Sz); VF >= MinVF;
+           VF /= 2) {
----------------
bjope wrote:
> With my OOT target I ended up here with getMinVecRegSize() returning 32.
> E->getMainOp was a load like this `%3 = load i24, i24* getelementptr inbounds ([128 x i24], [128 x i24]* @a_ua, i16 0, i16 3)`, so Sz was 24.
> 
> That gives a MinVF that is 0. And after some iterations the inner loop was entered with VF=0, which gives a slice that is empty, hitting assertions when doing Slice.front().
> 
> I haven't reduced this failure for any in-tree target (a bit short on people at the office here in the middle of summer). But maybe you should make sure MinVF doesn't go below 1 here (or maybe not even below 2) to avoid getting a VF that is less than 1 (or less than 2). Or check that VF is at least 1 (or 2) in the loop guard.
> 
> (I do not know if VF=1 makes any sense. Hence the alternatives above regarding 1 or 2.)
The fix is ready (see D107058), will commit it tomorrow. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105020