[PATCH] D17288: [CodeGenPrepare] Do select to branch transform when cmp's operand is expensive.

[Sanjay Patel via llvm-commits]

spatel added a comment.

In http://reviews.llvm.org/D17288#373170, @evandro wrote:

> I don't like the idea of FdivLatency having a fixed value in cycles.  Firstly, which FP division does this apply to?  Secondly, it fails to consider its throughput.  Thirdly, the default value may be convenient to some targets, but is far from universally acceptable.  Fourthly, it's not a good practice to have this value in one place and also elsewhere, like in the pipeline model, where the same information is richly described in all its variations.
>
> Rather, I'd be more comfortable with a simple boolean value indicating that divisions (not only FP ones) are expensive, defaulting to false, or a hook into the target to examine the instr and return the actual cycle count or whether it's indeed expensive.


These are good points. If we're going to use detailed models, we shouldn't duplicate just a single point from those models. We do have a simplified cost model in TTI for use at this level:
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Analysis/TargetTransformInfoImpl.h#L47

And this is what this patch used originally, but now we have introduced the branch mispredict penalty into the equation too.

By design I think, the TTI cost model is limited in the information it can provide. But it does simulate (quite poorly in some cases) information that exists in detail in the SchedMachineModel. Note that in PR26837:
https://llvm.org/bugs/show_bug.cgi?id=26837
...we've discussed making latency and throughput more explicit in the TTI cost model because that would be helpful for the vectorizer, inliner, unroller, and others. But I think that does raise the question: should the TTI cost model inherit its data from the more detailed SchedMachineModel instead of duplicating it?


http://reviews.llvm.org/D17288