<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<div class="moz-cite-prefix">On 03/15/2017 12:41 PM, Samuel Antão
wrote:<br>
</div>
<blockquote
cite="mid:CAK4VZWD6OXL7NoRmsfkTeboN_fO5QWHm572bpeeRgHvhwAznYA@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<div dir="ltr">Thank you all for the responses!
<div><br>
</div>
<div>I understand that FP exceptions are activated
programatically and can be disabled.</div>
</div>
</blockquote>
<br>
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.<br>
<br>
<blockquote
cite="mid:CAK4VZWD6OXL7NoRmsfkTeboN_fO5QWHm572bpeeRgHvhwAznYA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div> 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:</div>
<div><br>
</div>
<div>double a, b, c</div>
<div>...</div>
<div>if (a > b)</div>
<div> c = a/b</div>
<div><br>
</div>
<div>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. <br>
</div>
</div>
</blockquote>
<br>
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).<br>
<br>
<blockquote
cite="mid:CAK4VZWD6OXL7NoRmsfkTeboN_fO5QWHm572bpeeRgHvhwAznYA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
</div>
</blockquote>
<br>
You should be able to use the FENV_ACCESS pragma to turn these
on/off even within the same translation unit.<br>
<br>
-Hal<br>
<br>
<blockquote
cite="mid:CAK4VZWD6OXL7NoRmsfkTeboN_fO5QWHm572bpeeRgHvhwAznYA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Thanks again!</div>
<div>Samuel</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Mar 15, 2017 at 3:22 PM, Hal
Finkel <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">[+current
llvm-dev address]
<div class="HOEnZb">
<div class="h5"><br>
<br>
<br>
On 03/15/2017 09:23 AM, Hal Finkel wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Samuel,<br>
<br>
What are you taking about? ;)<br>
<br>
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.).<br>
<br>
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.<br>
<br>
(*) There's ongoing work to change that. Search the
mailing list for Andrew Kaylor.<br>
<br>
-Hal<br>
<br>
On 03/15/2017 04:38 AM, Samuel Antão wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi all,<br>
<br>
I came across an issue caused by the Call-Graph
Simplify Pass. Here is a a small repro:<br>
<br>
```<br>
define double @foo(double %a1, double %a2, double
%a3) #0 {<br>
entry:<br>
%a_mul = fmul double %a1, %a2<br>
%a_cmp = fcmp ogt double %a3, %a_mul<br>
br i1 %a_cmp, label %a.then, label %a.end<br>
<br>
a.then:<br>
%a_div = fdiv double %a_mul, %a3<br>
br label %a.end<br>
<br>
a.end:<br>
%a_factor = phi double [ %a_div, %a.then ], [
1.000000e+00, %entry ]<br>
ret double %a_factor<br>
}<br>
```<br>
Here, the conditional is guarding a possible
division by zero. However if I run CGSimplify on
this I get:<br>
```<br>
define double @foo(double %a1, double %a2, double
%a3) local_unnamed_addr #0 {<br>
entry:<br>
%a_mul = fmul double %a1, %a2<br>
%a_cmp = fcmp olt double %a_mul, %a3<br>
%a_div = fdiv double %a_mul, %a3<br>
%a_factor = select i1 %a_cmp, double %a_div,
double 1.000000e+00<br>
ret double %a_factor<br>
}<br>
```<br>
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::isSafeToSpeculativelyEx<wbr>ecute` 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):<br>
```<br>
diff --git a/lib/Analysis/ValueTracking.c<wbr>pp
b/lib/Analysis/ValueTracking.c<wbr>pp<br>
index 1761dac..c61fefd 100644<br>
--- a/lib/Analysis/ValueTracking.c<wbr>pp<br>
+++ b/lib/Analysis/ValueTracking.c<wbr>pp<br>
@@ -3352,6 +3352,21 @@ bool
llvm::isSafeToSpeculativelyExe<wbr>cute(const Value
*V,<br>
// The numerator *might* be MinSignedValue.<br>
return false;<br>
}<br>
+ case Instruction::FDiv:<br>
+ case Instruction::FRem:{<br>
+ const Value *Denominator =
Inst->getOperand(1);<br>
+ // x / y is undefined if y == 0<br>
+ // The denominator is not a constant, so there
is nothing we can do to prove<br>
+ // it is non-zero.<br>
+ if (auto *VV =
dyn_cast<ConstantVector>(Denom<wbr>inator))<br>
+ Denominator = VV->getSplatValue();<br>
+ if (!isa<ConstantFP>(Denominator)<wbr>)<br>
+ return false;<br>
+ // The denominator is a zero constant - we
can't speculate here.<br>
+ if (m_AnyZero().match(Denominator<wbr>))<br>
+ return false;<br>
+ return true;<br>
+ }<br>
case Instruction::Load: {<br>
const LoadInst *LI =
cast<LoadInst>(Inst);<br>
if (!LI->isUnordered() ||<br>
```<br>
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:<br>
<br>
- is there any target that would benefit from
speculative fp divisions?<br>
- is there any target for which fp division does not
have side effects?<br>
<br>
If not, I can go ahead and post the patch above for
review.<br>
<br>
Many thanks!<br>
Samuel<br>
</blockquote>
<br>
</blockquote>
<br>
-- <br>
Hal Finkel<br>
Lead, Compiler Technology and Programming Languages<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</body>
</html>