[PATCH] D103247: [SLP]Allow to reorder nodes with >2 scalar values.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7540
+        BoUpSLP::OrdersType NewOrder(Order->begin(), Order->end());
+        fixupOrderingIndices(NewOrder);
         // TODO: reorder tree nodes without tree rebuilding.
----------------
RKSimon wrote:
> All the calls to fixupOrderingIndices create a 'NewOrder' immediately beforehand - maybe just perform the copy inside fixupOrderingIndices and return it? Its up to you - I have no strong preference.
Yes, thought about it too, will check once again.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7970
   // extract scalars into scalar registers, so NeedExtraction is set true.
-  return tryToVectorizeList(BuildVectorOpds, R, /*AllowReorder=*/false);
 }
----------------
RKSimon wrote:
> tbh I'd prefer to see the AllowReorder arg retained (and made compulsory in tryToVectorizeList) to make it easier to grok.
Ok, will restore it.


================
Comment at: llvm/test/Transforms/SLPVectorizer/X86/alternate-calls-inseltpoison.ll:7
+; RUN: opt < %s -mtriple=x86_64-unknown -mcpu=knl -basic-aa -slp-vectorizer -instcombine -S | FileCheck %s --check-prefixes=AVX512
+; RUN: opt < %s -mtriple=x86_64-unknown -mcpu=skx -basic-aa -slp-vectorizer -instcombine -S | FileCheck %s --check-prefixes=AVX512
 
----------------
RKSimon wrote:
> I may be wrong but I think all the AVX modes can use the same AVX prefix?
Will check, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103247