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

[Kaylor, Andrew via llvm-dev]

It’s true, I am working on this.  I have committed the initial patch to add constrained intrinsics for the basic FP operations.  This has the desired effect of preventing optimizations that would violate strict FP semantics with regard to rounding mode and exception status, but it also prevents many safe optimizations.  Later this year I’ll be going through the code base and trying to teach various optimizations to handle the constrained intrinsics safely.

Nothing has been added to clang yet to generate these intrinsics, though I have some rough code to do so that’s just waiting for an implementation of the much harder task of figuring out when such restrictions are needed.  If anyone did have a front end that generated these intrinsics, everything is in place to get them through to code generation (though they currently become at least theoretically unrestricted again after ISel).  I have an experimental pass that converts all basic FP operations to the constrained versions and I’ve been able to successfully compile and run real-world FP-using applications with that pass in place.

I’m currently working on a patch to add constrained versions of the standard library FP intrinsics (sin, cos, pow, etc.).

If anyone is interested in getting pieces of the work-in-progress I’ve mentioned above to test drive, let me know.  I’d be happy to share.

-Andy


From: Sanjay Patel [mailto:spatel at rotateright.com]
Sent: Wednesday, March 15, 2017 8:16 AM
To: Samuel Antão <samuelfantao at gmail.com>; Kaylor, Andrew <andrew.kaylor at intel.com>
Cc: llvm-dev <llvm-dev at lists.llvm.org>; acjacob at us.ibm.com
Subject: Re: [llvm-dev] Speculative execution of FP divide Instructions - Call-Graph Simplify

> - is there any target for which fp division does not have side effects?

Yes - all of them. This goes back to the fact that clang/llvm do not support changing the FPENV:
https://bugs.llvm.org/show_bug.cgi?id=8100
There has been some effort to change that recently, so maybe this is (partly) fixed? (cc'ing Andy Kaylor)
These reports may also provide info / answers / work-arounds:
https://bugs.llvm.org/show_bug.cgi?id=6050
https://bugs.llvm.org/show_bug.cgi?id=24343
https://bugs.llvm.org/show_bug.cgi?id=24818

On Wed, Mar 15, 2017 at 3:41 AM, Samuel Antão via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
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

_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170315/cd00a6a1/attachment.html>