[PATCH] D105432: [Analysis] Add simple cost model for strict (in-order) reductions

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:2085
+  InstructionCost
+  getArithmeticStrictReductionCost(unsigned Opcode, VectorType *Ty,
+                                   TTI::TargetCostKind CostKind) {
----------------
Can you unify the interface for getArithmeticReductionCost to take an enum which specifies the kind of reduction we want: tree, pair-wise or in-order?
If there is only a single `getReductionCost` interface where the parameters dictate having to specify what cost is being asked for, than that's difficult to get wrong (you need to fill in the parameters), but if there's another slightly more specialized variant available, people may not realise it exists and just call the most basic cost interface instead.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:2090
+        getArithmeticInstrCost(Opcode, VTy->getElementType(), CostKind);
+    return Cost * VTy->getNumElements();
+  }
----------------
This seems to be missing the cost of extracting each of the elements which you can get using `getScalarizationOverhead(..., /*bool Extract=*/true, ...)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105432