[PATCH] D103247: [SLP]Allow to reorder nodes with >2 scalar values.
Simon Pilgrim via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 2 02:23:37 PDT 2021
RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.
LGTM with a few minors
================
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.
----------------
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.
================
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);
}
----------------
tbh I'd prefer to see the AllowReorder arg retained (and made compulsory in tryToVectorizeList) to make it easier to grok.
================
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
----------------
I may be wrong but I think all the AVX modes can use the same AVX prefix?
================
Comment at: llvm/test/Transforms/SLPVectorizer/X86/alternate-calls.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
----------------
I may be wrong but I think all the AVX modes can use the same AVX prefix?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103247/new/
https://reviews.llvm.org/D103247
More information about the llvm-commits
mailing list