[PATCH] D14185: Extend SLP Vectorizer to deal with aggregates

[Hal Finkel via llvm-commits]

hfinkel accepted this revision.
hfinkel added a reviewer: hfinkel.
hfinkel added a comment.

LGTM too.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:357-366
@@ -375,1 +356,12 @@
     CodeMetrics::collectEphemeralValues(F, AC, EphValues);
+    // Use the vector register size specified by the target unless overridden
+    // by a command-line option.
+    // TODO: It would be better to limit the vectorization factor based on
+    //       data type rather than just register size. For example, x86 AVX has
+    //       256-bit registers, but it does not support integer operations
+    //       at that width (that requires AVX2).
+    if (MaxVectorRegSizeOption.getNumOccurrences())
+      MaxVecRegSize = MaxVectorRegSizeOption;
+    else
+      MaxVecRegSize = TTI->getRegisterBitWidth(true);
+
----------------
Yes, please split this when committing. The refactoring should be separate.

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1512
@@ +1511,3 @@
+  if (ST) {
+    // Check that struct is homogeneous
+    for (const auto *Ty : ST->elements())
----------------
Comment should be a sentence that ends with a period.

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1537
@@ +1536,3 @@
+      return false;
+    // Check if load can be rewritten as load of vector
+    LoadInst *LI = dyn_cast<LoadInst>(Vec);
----------------
Comment should end with a period.


http://reviews.llvm.org/D14185