[PATCH] D12882: [SimplifyCFG] do not speculate fdiv by default; it's expensive (PR24818)

[Sanjay Patel via llvm-commits]

spatel created this revision.
spatel added reviewers: hfinkel, arsenm, bkramer.
spatel added a subscriber: llvm-commits.

I'm not sure if this is the right fix, but PR24818 and PR24343 show that we're now speculating costly ops on x86 when we shouldn't. I've confirmed that we're hoisting fdiv on all in-tree targets without this patch, and that seems wrong, so I think we should fix this in the base class rather than everyone's overrides.

The test just below the one I'm proposing assumes that fsqrt is cheap enough to speculate, but that's not generally true either. Do we want SimplifyCFG to make this speculative transform and then have backends undo it for expensive ops?

http://reviews.llvm.org/D12882

Files:
  include/llvm/Analysis/TargetTransformInfoImpl.h
  test/Transforms/SimplifyCFG/speculate-math.ll

Index: test/Transforms/SimplifyCFG/speculate-math.ll
===================================================================
--- test/Transforms/SimplifyCFG/speculate-math.ll
+++ test/Transforms/SimplifyCFG/speculate-math.ll
@@ -7,6 +7,24 @@
 declare float @llvm.minnum.f32(float, float) nounwind readonly
 declare float @llvm.maxnum.f32(float, float) nounwind readonly
 
+; Do not speculate fdiv by default because it is generally expensive. 
+
+; CHECK-LABEL: @fdiv_test(
+; CHECK-NOT: select
+define double @fdiv_test(double %a, double %b) {
+entry:
+  %cmp = fcmp ogt double %a, 0.0
+  br i1 %cmp, label %cond.true, label %cond.end
+
+cond.true:
+  %div = fdiv double %b, %a
+  br label %cond.end
+
+cond.end:
+  %cond = phi double [ %div, %cond.true ], [ 0.0, %entry ]
+  ret double %cond
+}
+
 ; CHECK-LABEL: @sqrt_test(
 ; CHECK: select
 define void @sqrt_test(float addrspace(1)* noalias nocapture %out, float %a) nounwind {
Index: include/llvm/Analysis/TargetTransformInfoImpl.h
===================================================================
--- include/llvm/Analysis/TargetTransformInfoImpl.h
+++ include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -61,6 +61,10 @@
       // Otherwise, the default basic cost is used.
       return TTI::TCC_Basic;
 
+    // FIXME: UDiv and SDiv should also default to expensive.
+    case Instruction::FDiv:
+      return TTI::TCC_Expensive;
+
     case Instruction::IntToPtr: {
       // An inttoptr cast is free so long as the input is a legal integer type
       // which doesn't contain values outside the range of a pointer.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D12882.34809.patch
Type: text/x-patch
Size: 1574 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150915/b8faea6d/attachment.bin>