[PATCH] Add support to recognize non SIMD kind of parallelism in SLPVectorizer
Arnold Schwaighofer
aschwaighofer at apple.com
Thu Jun 5 08:58:43 PDT 2014
Hi Karthik,
thanks for working on this! I have a few questions embedded in the source. Also, did you run the test suite for correctness and performance?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:152
@@ -151,1 +151,3 @@
+static unsigned checkAddSubInst(ArrayRef<Value *> VL) {
+ Instruction *I0 = dyn_cast<Instruction>(VL[0]);
----------------
Could we name this function "isAddSubInst" and add some documentation that we encode vector bundles that are combination of opcodes as a shufflevector.
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:492
@@ -473,2 +491,3 @@
for (int i = 0, e = VL.size(); i != e; ++i) {
- assert(!ScalarToTreeEntry.count(VL[i]) && "Scalar already in tree!");
+ assert(!(getSameOpcode(VL) != Instruction::ShuffleVector &&
+ ScalarToTreeEntry.count(VL[i])) &&
----------------
I don't understand why you are relaxing this constraint. Why should a scalar instruction be part of two vectors?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:765
@@ -734,3 +764,3 @@
for (unsigned j = i+1; j < e; ++j)
- if (VL[i] == VL[j]) {
+ if ((VL[i] == VL[j]) && (Opcode != Instruction::ShuffleVector)) {
DEBUG(dbgs() << "SLP: Scalar used twice in bundle.\n");
----------------
I don't understand why you are relaxing this constraint. Within an ADDSUB operation you still want the instructions to be unique?
For the bundle [ADD1, SUB1, ADD2, SUB2] we still want all the members to be unique in the bundle?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1231
@@ +1230,3 @@
+ VectorType *MaskTy = VectorType::get(Builder.getInt1Ty(), VL.size());
+ VecCost += TTI->getCastInstrCost(Opcode, VecTy, MaskTy);
+ return VecCost - ScalarCost;
----------------
I think we should use the getShuffleCost instruction (and we would need a new ShuffleKind and implement the cost model for it).
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1860
@@ +1859,3 @@
+ // If this Scalar was already handled then continue
+ if (ScalarEntry.count(Scalar))
+ continue;
----------------
Why is this needed?
http://reviews.llvm.org/D4015
More information about the llvm-commits
mailing list