[llvm-dev] Speculative execution of FP divide Instructions - Call-Graph Simplify
Hal Finkel via llvm-dev
llvm-dev at lists.llvm.org
Wed Mar 15 11:52:13 PDT 2017
On 03/15/2017 12:41 PM, Samuel Antão wrote:
> Thank you all for the responses!
>
> I understand that FP exceptions are activated programatically and can
> be disabled.
The point is that they should be disabled by default. If you enable
them, you need to tell the compiler you've done so (using a command-line
option or #pragma STC FENV_ACCESS ON). Otherwise, the compiler gets to
assume that they're off. We currently don't support anything else.
> I guess my point is that those divisions can have side effects and the
> compiler can't prove they have not. I imagine that, from a user
> viewpoint, if I write code like:
>
> double a, b, c
> ...
> if (a > b)
> c = a/b
>
> thinking that my conditional also guards against divisions by zero,
> and activate the exceptions exactly to assert that, transformations
> that speculate the execution of the divisions won't serve me well. The
> whole purpose of having FP exception support just vanishes.
When we support a mode where FP exceptions are enabled, we'll disable
such transformations (as we're currently working on doing this, with
special intrinsics for the FP operations in these modes, we get a lack
of this kind of speculation naturally).
>
> I guess that at the end of the day this is about choosing what is a
> "good" default behaviour. I don't feel strongly one way or the other,
> just thought that this deserved some discussion.
>
> I don't think this is what Andrew is trying to solve, is it?
> Exceptions can be activated anywhere, not necessarily in the same
> compilation unit.
You should be able to use the FENV_ACCESS pragma to turn these on/off
even within the same translation unit.
-Hal
>
> Thanks again!
> Samuel
>
> On Wed, Mar 15, 2017 at 3:22 PM, Hal Finkel <hfinkel at anl.gov
> <mailto:hfinkel at anl.gov>> wrote:
>
> [+current llvm-dev address]
>
>
>
> On 03/15/2017 09:23 AM, Hal Finkel wrote:
>
> Hi Samuel,
>
> What are you taking about? ;)
>
> The only way to get a SIGFPE from a floating-point division by
> zero is to modify the floating-point environment to enable
> those exceptions. We don't support that (*). In the default
> (supported) environment, floating point division is well
> defined for all inputs (dividing by 0 gives inf, by NaN gives,
> NaN, etc.).
>
> Regarding whether it makes sense to speculate, that's clearly
> a target-dependent property. On some targets this makes sense
> (e.g. OOO cores where divides have high, by hideable, latency)
> and on some targets it really doesn't. If we're speculating
> these on targets where we shouldn't, then we need to fix the
> cost model.
>
> (*) There's ongoing work to change that. Search the mailing
> list for Andrew Kaylor.
>
> -Hal
>
> On 03/15/2017 04:38 AM, Samuel Antão 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!
> Samuel
>
>
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>
--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170315/b0442dea/attachment.html>
More information about the llvm-dev
mailing list