[PATCH] D57779: [SLP] Add support for throttling.
Vasileios Porpodas via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 13 11:39:39 PDT 2019
vporpo added inline comments.
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:645
+ /// Save seed instructions to try partially vectorize later.
+ void recordSeeds(ArrayRef<Value *> Ops);
+
----------------
Why are we collecting the seeds specifically for partial vectorization? Is this really needed? Why don't you just call tryPartialVectorization(Seeds) within vectorizeStorChain() etc. ?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1278
+ /// True if this node has more than one non-gathering child.
+ bool IsBranch = false;
+
----------------
Do we need a separate variable for this? Can't we replace it with a small isBranch() function that checks the operands?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1297
+ /// The index containing the use of this entry.
+ SmallVector<TreeEntry *, 1> UseTreeIndices;
+
----------------
Hmm is this needed? Isn't this available in EdgeInfo of UserTreeIndices?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3584
+
+int BoUpSLP::getExtractCost() const {
+ int ExtractCost = 0;
----------------
Do we need separate getExtractCost() , getSpillCost() , getInsertCost() functions? Can't we update the getTreeCost() function instead to be aware of tree cutting?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3672
+
+void BoUpSLP::treeTraversal() {
+ // Find nodes with more than one use and it might include cycles because
----------------
I think we could avoid this traversal completely by marking the nodes when the tree is being built in buildTree_rec(). For example, the TE->ProposedToGather could be set in newTreeEntry().
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57779/new/
https://reviews.llvm.org/D57779
More information about the llvm-commits
mailing list