[PATCH] D12603: Use fixed-point representation for BranchProbability

[Cong Hou via llvm-commits]

congh added inline comments.

================
Comment at: test/CodeGen/AArch64/aarch64-deferred-spilling.ll:26
@@ -27,1 +25,3 @@
+; REGULAR-NOT: {{[wx]}}21{{,?}}
+; REGUAL: ldr w21, [sp, #[[OFFSET]]] // 4-byte Folded Reload
 ;
----------------
davidxl wrote:
> register name change?
Fixed. This test case keeps unchanged after the patch update. See explains below.

================
Comment at: test/CodeGen/ARM/cmpxchg-weak.ll:11
@@ -10,3 +10,3 @@
 ; CHECK:     cmp     [[LOADED]], r1
-; CHECK:     strexeq [[SUCCESS:r[0-9]+]], r2, [r0]
-; CHECK:     cmpeq   [[SUCCESS]], #0
+; CHECK:     strex   [[SUCCESS:r[0-9]+]], r2, [r0]
+; CHECK:     cmp     [[SUCCESS]], #0
----------------
davidxl wrote:
> unrelated change?
This change is caused by fixed point probability representation. After investigating the root cause, I found that it is from if-conversion. More specifically, when determine whether to do if-conversion there is a cost model measurement in which the branch probability is used to scale some cost (in integers). The current scale method does not round the result, which may be a problem. For example, ARMBaseInstrInfo::isProfitableToIfCvt() has the following statements:


```
  unsigned TUnpredCost = Probability.scale(TCycles);
  unsigned FUnpredCost = Probability.getCompl().scale(FCycles);
  unsigned UnpredCost = TUnpredCost + FUnpredCost;
```

Now assume Probability is 0.5 and both TCycles and FCycles are 1. If scale() does not round, then TUnpredCost and FUnpredCost are 0, and hence UnpredCost = TUnpredCost + FUnpredCost is also zero. This doesn't make sense as here if we calculate the cost in floating point we should get ~1 as UnpredCost.

We we do round in scale(), it is possible to get both TUnpredCost and FUnpredCost as 1, which is also incorrect. However, in our fixed point representation, 0.5 is actually a little less than 0.5, and its complement is a little greater than 0.5. Therefore we will have TUnpredCost = 0, and FUnpredCost = 1 after rounding. This is very hacky. 

So I think we do need scale() to have rounding behavior, but in ARMBaseInstrInfo::isProfitableToIfCvt() to avoid precision related issues it is better to scale TCycles/FCycles up (e.g. x1000) before scaling them by Probability. This is find as the function returns the value of 


```
return (TCycles + FCycles + TExtra + FExtra) <= UnpredCost;

```
and we can scale every operands up.

What do you think?


http://reviews.llvm.org/D12603