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

[Junmo Park via llvm-commits]

flyingforyou added inline comments.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4525
@@ +4524,3 @@
+    if (I && I->getOpcode() == Instruction::FDiv &&
+        STI->getSchedModel().FdivLatency >
+            STI->getSchedModel().MispredictPenalty)
----------------
Gerolf wrote:
> It that really a good heuristic? Even when the divide latency is less than or equal to the branch mispredication penalty issuing a branch can be the better choice. That depends on the program behavior. I believe the reasoning you are looking for is this: in the presence of a long latency instruction assume the dependent branch is well predicted most of the time. Practically the long latency of the divide covers for the (dynamic) instances when that assumption is wrong. 
> Even when the divide latency is less than or equal to the branch mispredication penalty issuing a branch can be the better choice. That depends on the program behavior. 
I also agree with this idea.. But what we can do for this in this patch?


> It that really a good heuristic? 
If you think this is not good, what heuristic do you recommend?



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4532
@@ +4531,3 @@
+
+  if (IsExpensiveCostInst(CmpOp0) || IsExpensiveCostInst(CmpOp1))
+    return true;
----------------
Gerolf wrote:
> In the case both paths consume the long latency select is still the better choice.
Why do you think so?

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4538
@@ -4513,3 +4537,3 @@
   // probably another cmov or setcc around, so it's not worth emitting a branch.
-  if (!Cmp || !Cmp->hasOneUse())
+  if (!Cmp->hasOneUse())
     return false;
----------------
Gerolf wrote:
> Why? The cmp could feed multiple selects from PHI nodes and still a branch would be preferable.
@bkramer or Sanjay, How do you think about this?

================
Comment at: test/Transforms/CodeGenPrepare/X86/select.ll:145
@@ +144,3 @@
+  %cmp = fcmp ogt float %div, %b
+  %sel = select i1 %cmp, float %div, float 8.0
+  ret float %sel
----------------
Gerolf wrote:
> I find this example misleading. The use of the %div in the select is irrelevant. The only issue is whether or not the branch is predictable.
Will fix this.


http://reviews.llvm.org/D17288