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

Wed Mar 15 11:18:29 PDT 2017

I'd imagine it's very difficult for compiler to track FP exception control
(unless it actually inserts fegetenv calls to get control bits at runtime
:/).
I think Hal was pointing out that straight-lining fp division is very rare
thing given that it is very expensive in many target architectures. It
probably means that cost model is whacky somewhere.

Kevin

On Wed, Mar 15, 2017 at 10:41 AM, Samuel Antão via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Thank you all for the responses!
>
> I understand that FP exceptions are activated programatically and can be
> disabled. 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.
>
> 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.
>
> Thanks again!
> Samuel
>
> On Wed, Mar 15, 2017 at 3:22 PM, Hal Finkel <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
>>
>>
>
> _______________________________________________
> LLVM Developers mailing list
> 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/feef59f7/attachment.html>