[PATCH] D115757: [SLP]Generalize cost model.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6313
+          ScalarCost = VL.size() *
+                       ScalarEltCost(std::distance(VL.begin(), find(VL, VL0)));
+        } else {
----------------
Is this purely compile time saving?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6341
+        Value *Op = PHI->getIncomingValue(I);
+        Operands[I] = Op;
       }
----------------
Can't we directly initialize Operands from PHI-> incoming_values()? Or even create a ArrayRef<> from it?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6389
+            return TTI->getVectorInstrCost(Instruction::ExtractElement,
+                                           SrcVecTy, *getExtractIndex(I));
+          },
----------------
We're going to lose the recent scalarization cost improvement here :-( 


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6621
+            unsigned OpIdx =
+                isa<BinaryOperator, GetElementPtrInst>(VL0) ? 1 : 0;
+            TTI::OperandValueInfo Op1Info = getOperandInfo(VL, 0);
----------------
Would isa<UnaryOperator> ? 0 : 1 be better?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6675
+            if (getOperandInfo(VL, 0).isConstant())
+              OpVK = TTI::OK_NonUniformConstantValue;
+            return TTI->getMemoryOpCost(Instruction::Store, VecTy,
----------------
Why do you avoid allowing uniform as well? Can't you just use getOperandInfo(VL, 0)..getNoProps() below ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115757/new/

https://reviews.llvm.org/D115757