Hal Finkel via llvm-dev
2017-Mar-15 15:22 UTC
[llvm-dev] Speculative execution of FP divide Instructions - Call-Graph Simplify
[+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
Samuel Antão via llvm-dev
2017-Mar-15 17:41 UTC
[llvm-dev] Speculative execution of FP divide Instructions - Call-Graph Simplify
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 > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170315/91505b50/attachment.html>
Flamedoge via llvm-dev
2017-Mar-15 18:18 UTC
[llvm-dev] Speculative execution of FP divide Instructions - Call-Graph Simplify
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>
Hal Finkel via llvm-dev
2017-Mar-15 18:52 UTC
[llvm-dev] Speculative execution of FP divide Instructions - Call-Graph Simplify
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>
Reasonably Related Threads
- Speculative execution of FP divide Instructions - Call-Graph Simplify
- Speculative execution of FP divide Instructions - Call-Graph Simplify
- [LLVMdev] More careful treatment of floating point exceptions
- UB and known bits
- [LLVMdev] possible addrspacecast problem