[PATCH] D110978: [SLP]Improve gathering of the scalars used in the graph.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:296
+  for (unsigned I = 0,
+                E = cast<FixedVectorType>(C->getType())->getNumElements();
+       I != E; ++I) {
----------------
Is it worth merging the isa<> and cast<> into a dyn_cast<>? 


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:494
+  VL.swap(SavedVL);
+  return Res;
+}
----------------
return None instead to make it obvious it failed?

Maybe do this as an early out instead of the much bigger if (Res.hasValue()) indented block?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4572
+        // FIXME: Remove this check after correct support for
+        // SK_ExtractSubvector is landed for all targets.
+        if (Idx % NumElts == 0)
----------------
What targets are we still missing support for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110978