<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 8, 2019 at 7:08 AM Szabolcs Nagy <<a href="mailto:nsz@port70.net" target="_blank">nsz@port70.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">* Sanjay Patel via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> [2019-10-01 09:44:54 -0400]:<br>
> Let's change the example to eliminate suspects:<br>
> #include <math.h><br>
> int is_nan(float x) {<br>
> /*<br>
> The following subclauses provide macros that are quiet (non<br>
> floating-point exception raising)<br>
> versions of the relational operators, and other comparison macros<br>
> that facilitate writing<br>
> efficient code that accounts for NaNs without suffering the<br>
> ‘‘invalid’’ floating-point exception.<br>
> */<br>
> return isunordered(x, x);<br>
> }<br>
> <br>
> The comment text is from 7.12.14 of the C standard draft. I'm hoping to<br>
> avoid any scenario under which it is ok to raise an exception in that code<br>
> (eliminate any questions about the clang front-end behavior / FENV_ACCESS).<br>
<br>
isunordered (like isnan) is a different operation from x!=x,<br>
in particular it does not signal even on signaling nans<br>
while x!=x does.<br>
<br>
> <br>
> As IR from clang with no optimization, this becomes a bunch of load/store<br>
> with:<br>
> %cmp = fcmp uno double %conv, %conv1<br>
> <br>
> Ok, so far? "fcmp uno" - <a href="http://llvm.org/docs/LangRef.html#fcmp-instruction" rel="noreferrer" target="_blank">http://llvm.org/docs/LangRef.html#fcmp-instruction</a><br>
> :<br>
> uno: yields true if either operand is a QNAN.<br>
<br>
why is that ok?<br></blockquote><div><br></div><div>Because there are no FP exceptions/signals for this IR opcode:</div><div><a href="http://llvm.org/docs/LangRef.html#floating-point-environment" target="_blank">http://llvm.org/docs/LangRef.html#floating-point-environment</a></div><div><br></div><div>AFAIK, the problem has been resolved with:</div><div><a href="https://reviews.llvm.org/D68463">https://reviews.llvm.org/D68463</a> (committed at r374025)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
here you need an unordered-not-equal, but got an unordered-compare,<br>
the former is a silent ieee754 operation the latter is signaling.<br>
<br>
as noted earlier this is wrong for signaling nans, because any fcmp<br>
would signal for them, but the lack of snan support can be forgiven,<br>
however here even qnans fail to remain silent, that's a bug.<br>
<br>
> <br>
> EarlyCSE/InstCombine reduce that fcmp to:<br>
> %cmp = fcmp uno float %x, 0.000000e+00<br>
> <br>
> Still good? Same fcmp predicate, but we replaced a repeated use of "%x"<br>
> with a zero constant to aid optimization.<br>
<br>
no, this is different from even x!=x not to mention isunordered(x,x).<br>
<br>
clang does not support FENV_ACCESS but repeatedly claimed in relevant<br>
bug reports that users who care should use -O0, but it seems to create<br>
broken compares even at -O0, so there is no way to get c99 conforming<br>
behaviour.<br></blockquote><div><br></div><div>I understand why someone may have suggested -O0 as a work-around, but as you've noted, that's not a complete solution to the problem. You likely need:</div><div><a href="http://llvm.org/docs/LangRef.html#constrainedfp">http://llvm.org/docs/LangRef.html#constrainedfp</a></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> <br>
> Now, send the optimized IR to codegen:<br>
> define i32 @is_nan(float %x) {<br>
> %cmp = fcmp uno float %x, 0.000000e+00<br>
> %r = zext i1 %cmp to i32<br>
> ret i32 %r<br>
> }<br>
> <br>
> $ llc -o - fpexception.ll -mtriple=armv7a<br>
> vmov s0, r0<br>
> mov r0, #0<br>
> vcmpe.f32 s0, s0<br>
> vmrs APSR_nzcv, fpscr<br>
> movwvs r0, #1<br>
> bx lr<br>
> <br>
> We produced "vcmpe" for code that should never cause an FP exception. ARM<br>
> codegen bug?<br>
<br>
sorry, the arm code gen is right here, the bug is in clang.<br>
<br>
> <br>
> On Tue, Oct 1, 2019 at 5:45 AM Kristof Beyls <<a href="mailto:Kristof.Beyls@arm.com" target="_blank">Kristof.Beyls@arm.com</a>> wrote:<br>
> <br>
> > Hi,<br>
> ><br>
> > I’ve been investigating <a href="https://bugs.llvm.org/show_bug.cgi?id=43374" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=43374</a>,<br>
> > which is about clang/llvm producing code that triggers a floating point<br>
> > exception when x is NaN, when targeting ARM, in the below code example.<br>
> ><br>
> > int bar(float x) {<br>
> > return x!=x ? 0 : 1;<br>
> > }<br>
> ><br>
> > The C99 standard states in section 7.12.14:<br>
> ><br>
> > """<br>
> > The relational and equality operators support the usual mathematical<br>
> > relationships between numeric values. For any ordered pair of numeric<br>
> > values exactly one of the relationships — less, greater, and equal — is<br>
> > true. Relational operators may raise the ‘‘invalid’’ floating-point<br>
> > exception when argument values are NaNs.<br>
> > """<br>
> ><br>
> > My interpretation of that paragraph is that it's OK for <, <=, > and >= to<br>
> > raise an exception when argument values are NaNs. It is not OK for == an !=<br>
> > to raise an exception when argument values are NaNs.<br>
> ><br>
> > Therefore,<br>
> ><br>
> > int bar(float x) {<br>
> > return x!=x ? 0 : 1;<br>
> > }<br>
> ><br>
> > should not produce an exception when x is NaN, and hence a vcmp rather<br>
> > than vcmpe instruction should be produced when generating ARM code for<br>
> > this.<br>
> ><br>
> > <a href="http://llvm.org/viewvc/llvm-project?rev=294945&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=294945&view=rev</a> introduced<br>
> > support for generating vcmp instead of vcmpe for equality comparisons.<br>
> > How come vcmpe is generated for (x!=x)?<br>
> ><br>
> > The answer is that InstCombine transforms the equality comparison into an<br>
> > "ordered comparison”. Before InstCombine:<br>
> > define dso_local i32 @bar(float %x) local_unnamed_addr {<br>
> > entry:<br>
> > %cmp = fcmp une float %x, %x<br>
> > %cond = select i1 %cmp, i32 0, i32 1<br>
> > ret i32 %cond<br>
> > }<br>
> ><br>
> > After InstCombine:<br>
> > define dso_local i32 @bar(float %x) local_unnamed_addr #0 {<br>
> > entry:<br>
> > %cmp = fcmp ord float %x, 0.000000e+00<br>
> > %cond = zext i1 %cmp to i32<br>
> > ret i32 %cond<br>
> > }<br>
> ><br>
> > Please note that on other backends like x86 or AArch64, this InstCombine<br>
> > doesn’t trigger floating point exception behaviour since those backends<br>
> > don’t seem to be producing any instructions for fcmp that raise floating<br>
> > point exceptions on NaNs.<br>
> ><br>
> > My question here is: how to fix this behaviour? Or: which part in the<br>
> > compilation flow is wrong?<br>
> > Reading through various standards and specifications, I’m getting confused<br>
> > to what the best fix would be:<br>
> ><br>
> ><br>
> > - <a href="https://llvm.org/docs/LangRef.html#floating-point-environment" rel="noreferrer" target="_blank">https://llvm.org/docs/LangRef.html#floating-point-environment</a> states<br>
> > "The default LLVM floating-point environment assumes that<br>
> > floating-point instructions do not have side effects. Results assume the<br>
> > round-to-nearest rounding mode. No floating-point exception state is<br>
> > maintained in this environment. Therefore, there is no attempt to create or<br>
> > preserve invalid operation (SNaN) or division-by-zero exceptions.”<br>
> > This suggests that if we want to retain floating point exception<br>
> > behaviour in the compilation flow, we shouldn’t be using the “default LLVM<br>
> > floating-point environment”, but rather something else. Presumably the<br>
> > constrained intrinsics? However, when I look at the constrained intrinsics<br>
> > definition, it seems (<br>
> > <a href="http://llvm.org/docs/LangRef.html#constrained-floating-point-intrinsics" rel="noreferrer" target="_blank">http://llvm.org/docs/LangRef.html#constrained-floating-point-intrinsics</a>)<br>
> > there is no constrained intrinsic for the floating point comparison<br>
> > operation. Should there be one?<br>
> > - If the default floating-point environment assumes that<br>
> > floating-point instructions do not have side effects, why does the Arm<br>
> > backend lower floating point comparison to vcmpe rather than vcmp? The<br>
> > revision history suggests this has been this way since the initial creation<br>
> > of the ARM backend. Should this behaviour be changed and vcmp be produced<br>
> > rather than vcmpe? And only later, once the generation of constrained<br>
> > floating point intrinsics is implemented should backends start producing<br>
> > signalling floating point comparisons for floating point comparison<br>
> > constrained intrinsics (assuming they’ll exist by then)?<br>
> > - Or alternatively, there is a good reason to keep on producing vcmpe<br>
> > as is today, and instcombine just shouldn’t convert “fcmp une” into “fcmp<br>
> > ord”?<br>
> > - Or as yet another alternative, instcombine is just fine converting<br>
> > “fcmp une” into “fcmp ord”, and it’s the ARM backend that should produce<br>
> > vcmp rather than vcmpe also for “unordered” comparisons, next to equality<br>
> > comparisons?<br>
> ><br>
> ><br>
> > Thanks,<br>
> ><br>
> > Kristof<br>
> ><br>
<br>
> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br>
</blockquote></div></div>