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

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 2 10:28:34 PDT 2017


RKSimon added a reviewer: RKSimon.
RKSimon added a comment.

Some minor style comments



================
Comment at: lib/Analysis/CostModel.cpp:202
+  Value *L = nullptr;
+  Value *R = nullptr;
+  if (m_BinOp(m_Value(L), m_Value(R)).match(I))
----------------
No need to initialize these. 
```
Value *L, *R;
```


================
Comment at: lib/Analysis/CostModel.cpp:204
+  if (m_BinOp(m_Value(L), m_Value(R)).match(I))
+    return {I->getOpcode(), L, R};
+  return ReductionData();
----------------
Use the ReductionData constructor for clarity?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4353
+    unsigned getFirstOperandIndex() const {
+      assert(*this);
+      return 0;
----------------
?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4358
+    unsigned getNumberOfOperands() const {
+      assert(!IsReducedValue && *this && LHS && RHS);
+      return 2;
----------------
Add assert message


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4363
+    unsigned getRequiredNumberOfUses() const {
+      assert(!IsReducedValue && *this && LHS && RHS);
+      return 1;
----------------
Add assert message


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4368
+    bool isAssociative(Instruction *I) const {
+      assert(!IsReducedValue && *this && LHS && RHS);
+      return I->isAssociative();
----------------
Add assert message


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4394
+    unsigned getOpcode() const {
+      assert(isVectorizable());
+      return Opcode;
----------------
Add assert message


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4445
+    if (m_BinOp(m_Value(LHS), m_Value(RHS)).match(V))
+      return {cast<BinaryOperator>(V)->getOpcode(), LHS, RHS};
+    return OperationData(V);
----------------
Use OperationData constructor for clarity


https://reviews.llvm.org/D29402





More information about the llvm-commits mailing list