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

[Junmo Park via llvm-commits]

flyingforyou added a comment.

Hi Evandro.

I really appreciate your comment. There is a history about why I choose this way. Please refer to previous patch. (http://reviews.llvm.org/D16836).

> I don't like the idea of FdivLatency having a fixed value in cycles. Firstly, which FP division does this apply to?


This can be easily handled by changing name. Now, I only consider that single precision fp division which has  mininum cycles.

> Secondly, it fails to consider its throughput.


Now, there is no way to show its throughput. But we don't need to consider it's throughput in this heuristic.

> 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.


I think these reasons are not only related with div-cmp-sel heuristic. I think we can talk about this issue on PR26837 or llvm-dev mail thread.

> 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.


Now, divisions are treated as expensive by default, likes below. Why do you think divisons are treated as cheap? If there are good reasons, we can talk about this also through llvm-dev.

  enum TargetCostConstants {
    TCC_Free = 0,     ///< Expected to fold away in lowering.
    TCC_Basic = 1,    ///< The cost of a typical 'add' instruction.
    TCC_Expensive = 4 ///< The cost of a 'div' instruction on x86.
  };
  
  unsigned getOperationCost(unsigned Opcode, Type *Ty, Type *OpTy) {
    switch (Opcode) {
    default:
      // By default, just classify everything as 'basic'.
      return TTI::TCC_Basic;
  
    case Instruction::FDiv:
    case Instruction::FRem:
    case Instruction::SDiv:
    case Instruction::SRem:
    case Instruction::UDiv:
    case Instruction::URem:
      return TTI::TCC_Expensive;


http://reviews.llvm.org/D17288