[llvm-dev] Speculative execution of FP divide Instructions - Call-Graph Simplify

[Samuel Antão via llvm-dev]

Hi all,

I came across an issue caused by the Call-Graph Simplify Pass. Here is a a
small repro:

```
define double @foo(double %a1, double %a2, double %a3) #0 {
entry:
  %a_mul = fmul double %a1, %a2
  %a_cmp = fcmp ogt double %a3, %a_mul
  br i1 %a_cmp, label %a.then, label %a.end

a.then:
  %a_div = fdiv double %a_mul, %a3
  br label %a.end

a.end:
  %a_factor = phi double [ %a_div, %a.then ], [ 1.000000e+00, %entry ]
  ret double %a_factor
}
```
Here, the conditional is guarding a possible division by zero. However if I
run CGSimplify on this I get:
```
define double @foo(double %a1, double %a2, double %a3) local_unnamed_addr
#0 {
entry:
  %a_mul = fmul double %a1, %a2
  %a_cmp = fcmp olt double %a_mul, %a3
  %a_div = fdiv double %a_mul, %a3
  %a_factor = select i1 %a_cmp, double %a_div, double 1.000000e+00
  ret double %a_factor
}
```
This will cause a FP arithmetic exception, and the application will get a
SIGFPE signal. The code that drives the change in the optimizer relies on
`llvm::isSafeToSpeculativelyExecute` to decide whether the division should
be performed speculatively. Right now, this function returns true. One
could do something similar to integer divisions and add a case there (this
solved the issue for me):
```
diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp
index 1761dac..c61fefd 100644
--- a/lib/Analysis/ValueTracking.cpp
+++ b/lib/Analysis/ValueTracking.cpp
@@ -3352,6 +3352,21 @@ bool llvm::isSafeToSpeculativelyExecute(const Value
*V,
     // The numerator *might* be MinSignedValue.
     return false;
   }
+  case Instruction::FDiv:
+  case Instruction::FRem:{
+    const Value *Denominator = Inst->getOperand(1);
+    // x / y is undefined if y == 0
+    // The denominator is not a constant, so there is nothing we can do to
prove
+    // it is non-zero.
+    if (auto *VV = dyn_cast<ConstantVector>(Denominator))
+      Denominator = VV->getSplatValue();
+    if (!isa<ConstantFP>(Denominator))
+      return false;
+    // The denominator is a zero constant - we can't speculate here.
+    if (m_AnyZero().match(Denominator))
+      return false;
+    return true;
+  }
   case Instruction::Load: {
     const LoadInst *LI = cast<LoadInst>(Inst);
     if (!LI->isUnordered() ||
```
I did my tests using a powerpc64le target, but I couldn't find any target
specific login involved in this transform. In any case, I wanted to drop
the questions:

- is there any target that would benefit from speculative fp divisions?
- is there any target for which fp division does not have side effects?

If not, I can go ahead and post the patch above for review.

Many thanks!
Sam
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170315/6d7bd2b9/attachment.html>