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

[Ayal Zaks via llvm-commits]

Ayal added a subscriber: Ayal.

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:280
@@ -299,4 +279,3 @@
     ConstantInt *CI = dyn_cast<ConstantInt>(E->getOperand(1));
-
-    if (!CI || CI->getZExtValue() != i || E->getOperand(0) != Vec)
-      return false;
+    return CI && CI->getZExtValue() == i;
+  } else {
----------------
maybe better to wrap return value inside ( )

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:283
@@ +282,3 @@
+    assert(Opcode == Instruction::ExtractValue);
+    ExtractValueInst *EI = cast<ExtractValueInst>(E);
+    assert(EI->getNumIndices()==1);
----------------
cast should be doing the assert above it.

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:284
@@ +283,3 @@
+    ExtractValueInst *EI = cast<ExtractValueInst>(E);
+    assert(EI->getNumIndices()==1);
+    return *EI->idx_begin()==i;
----------------
hfinkel wrote:
> You assert this here, but where is it checked?
Should check similar to the way GEPs are checked to be 2 indexed.
Also spaces around ==

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1486
@@ +1485,3 @@
+  }
+  if (!VectorType::isValidElementType(EltT))
+    return 0;
----------------
better call local isValidElementType to catch FP80, FP128 fwiw.

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1489
@@ +1488,3 @@
+  uint64_t VTSize = DL.getTypeSizeInBits(VectorType::get(EltT, N));
+  if (VTSize < MinVecRegSize || VTSize>MaxVecRegSize || VTSize != DL.getTypeSizeInBits(T))
+    return 0;
----------------
spaces around >

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1502
@@ +1501,3 @@
+
+bool BoUpSLP::canReuseExtract(ArrayRef<Value *> VL, unsigned Opcode) const {
+  assert(Opcode == Instruction::ExtractElement ||
----------------
may be simpler to obtain Opcode from getSameOpcode(VL), instead of passing it.

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2329
@@ +2328,3 @@
+        return propagateMetadata(V, E->Scalars);
+      }
+    }
----------------
else ... return Gather? (but w/o explicit 'else')


http://reviews.llvm.org/D14185