[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