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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7215-7223
+    // Let TTI model the extra cost in case of a branch before a predicated
+    // block, by passing VF in such cases.
+    unsigned PredicatedBBVF = 0;
+    BranchInst *BI = cast<BranchInst>(I);
+    if (VF > 1 && BI->isConditional() &&
+        (PredicatedBBsAfterVectorization.count(BI->getSuccessor(0)) ||
+         PredicatedBBsAfterVectorization.count(BI->getSuccessor(1))))
----------------
mssimpso wrote:
> We may want to model the branch cost more carefully *inside the vectorizer* for the predication and if-conversion cases. It doesn't look like we model this at all yet.
> 
> I think there are 3 cases to consider: (1) the back-edge branch, (2) branches that are if-converted, and (3) branches that are unrolled/replicated due to predication. I think the current cost for Br is enough for (1). For (2) the branch goes away so the cost would be zero. But we should also think about the the cost of the selects introduced for if-conversion too at some point (there's a TODO for this with the PHI cost). For (3), I imagine we would, at least initially, model the replicated branches similar to the way we model scalarized instructions. Something like VF * TTI.getCFInstrCost().
I think all these cases can be handled with the use of the new PredicatedBBVF variable, or?

1) Back-edge branch: PredicatedBBVF==0
2) if-converted branches: PredicatedBBVF==0
3) Unrolled branches: PredicatedBBVF==VF.

So you are suggesting that getCFInstrCost(Instruction::Br) is called for the cost of a branch instruction, and that the LoopVectorizer should here instead decide when to call it, rather than passing PredicatedBBVF to it?



https://reviews.llvm.org/D31175