[PATCH] D29402: [SLP] Initial rework for min/max horizontal reduction vectorization, NFC.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: lib/Analysis/CostModel.cpp:211
+  Value *R;
+  unsigned Opcode = getReductionOpcode(I, L, R);
+  if (!Opcode)
----------------
Can PatternMatch be put to more use, by asking it to match a BinOp whose L and R sides are both shuffles, when Level > 0?

Passing L and R by reference here saves little compared to the original direct calls to getOperand(0) and getOperand(1), and seems mostly redundant elsewhere.


================
Comment at: lib/Analysis/CostModel.cpp:405
 
-    RdxOp = NextRdxOp;
+    RdxOp = dyn_cast<Instruction>(NextRdxOp);
     NumVecElemsRemain /= 2;
----------------
Check if RdxOp, or use cast instead of dyn_cast?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4094
 
+  if (!isa<BinaryOperator>(V))
+    return false;
----------------
V is-surely-a<BinaryOperator>, being a non-nullptr to BinaryOperator, right?


https://reviews.llvm.org/D29402