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

[Hal Finkel via llvm-dev]

[+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