[PATCH] D31175: Improve TargetTransformInfo::getCFInstrCost()

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7223-7228
+    if (ScalarPredicatedBB) {
+      // Return cost for branches around scalarized and predicated blocks.
+      Type *Vec_i1Ty =
+          VectorType::get(IntegerType::getInt1Ty(RetTy->getContext()), VF);
+      return (TTI.getScalarizationOverhead(Vec_i1Ty, false, true) +
+              (TTI.getCFInstrCost(Instruction::Br) * VF));
----------------
mssimpso wrote:
> We should probably also include the cost of the unconditional branches inside the predicated blocks. This would amount to another factor of VF * TTI.getCFInstrCost(Instruction::Br) / getReciprocalPredBlockProb() for the ScalarPredicatedBB case.
Regarding my previous comment, this should probably be a separate case. Something like:

```
if (VF > 1 && Legal->blockNeedsPredication(BI->getParent())
  return  VF * TTI.getCFInstrCost(Instruction::Br) / getReciprocalPredBlockProb();
```


================
Comment at: test/Analysis/CostModel/SystemZ/branch-predicated-vectorized-block.ll:1
+; RUN: opt < %s -cost-model -analyze -mtriple=systemz-unknown -mcpu=z13 | FileCheck %s
+;
----------------
It would be better to test this patch with the loop-vectorizer pass and the pre-vectorized loop as input. That way we can verify that the cost estimate accurately reflects the code that will be generated for different vectorization factors.


https://reviews.llvm.org/D31175