[PATCH] D30649: [SLP] Function for instruction cost calculation, NFC.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:420
+  /// VecTy are specified).
+  Optional<int> getCostOrDiff(unsigned Opcode, ArrayRef<Value *> VL,
+                              Type *ScalarTy, Type *VecTy = nullptr) const;
----------------
mkuper wrote:
> This seems like a weird interface, for a couple of reasons:
> 
> 1) It's somewhat vague - I'd expect different APIs to get a cost and to get the diffs.
> 2) It returns an Optional, but none of the uses actually check the returned value is valid. And when would you expect to return None anyway? Shouldn't we recognize any instruction here? If not, then do we expect to assert, or just silently decide the instruction will be scalarized?
> 3) Only the Call case actually wants the scalar and the vector costs separately, and I don't really understand why - it ends up subtracting them.
> 
> Splitting out a function that takes the vector and scalar type, and returns the difference, instead of having the subtraction repeated in every case, doesn't sound like a bad idea. Can you explain why this is an improvement over that, though?
Michael, I reworked it. It is required for another path I'm going to publish today.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1929
       CallInst *CI = cast<CallInst>(VL0);
-      Intrinsic::ID ID = getVectorIntrinsicIDForCall(CI, TLI);
-
-      // Calculate the cost of the scalar and vector calls.
-      SmallVector<Type*, 4> ScalarTys, VecTys;
-      for (unsigned op = 0, opc = CI->getNumArgOperands(); op!= opc; ++op) {
-        ScalarTys.push_back(CI->getArgOperand(op)->getType());
-        VecTys.push_back(VectorType::get(CI->getArgOperand(op)->getType(),
-                                         VecTy->getNumElements()));
-      }
-
-      FastMathFlags FMF;
-      if (auto *FPMO = dyn_cast<FPMathOperator>(CI))
-        FMF = FPMO->getFastMathFlags();
-
-      int ScalarCallCost = VecTy->getNumElements() *
-          TTI->getIntrinsicInstrCost(ID, ScalarTy, ScalarTys, FMF);
+      int ScalarCallCost = getCostOrDiff(Opcode, VL, ScalarTy).getValue();
 
----------------
mkuper wrote:
> Isn't the point of getCostOrDiff that you should be able to pass the vector and the scalar types, and get the difference? Why are you using two calls and then subtracting?
I reworked this code 


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4981
 
+  SmallVector<WeakVH, 4> ExtractInsts;
   for (BasicBlock::iterator it = BB->begin(), e = BB->end(); it != e; it++) {
----------------
mkuper wrote:
> This looks unrelated.
Yes, part of another patch, will be removed


https://reviews.llvm.org/D30649