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

Matthew Simpson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 5 06:53:06 PDT 2017


mssimpso added a comment.

In https://reviews.llvm.org/D31175#718819, @jonpa wrote:

> > This is a fragile test case that we should eventually fix. The cost of 3 for the branch would make sense if the condition were loop-varying, but in this case, the extracts will be removed by InstCombine, so the branch cost should be 0 (no scalarization overhead). I think we either want to modify getScalarizationOverhead to only compute a cost for loop-varying values or to check that the condition is loop-varying before calling getScalarizationOverhead.
>
> I see. That makes sense to me, and I actually also have a patch in progress to handle loop invariant and constant values in LoopVectorizer. However, given that I have a big patch for SystemZ cost functions nearly commited, which depend on this and and at least two more patches, I wonder if we could temporarily make this an XFAIL until the other patches have been evaluated and commited? I suspect that handling loop invariants and constants will have some impact on benchmarks and probably some regressions that must then be handled...


I'd prefer to not XFAIL a test. We should go ahead and make it more robust while we're looking at it. I can take a stab at this today since I think I originally wrote this test.


https://reviews.llvm.org/D31175





More information about the llvm-commits mailing list