Summary: | ARM soft-float extendsfdf2 fails to quiet signaling NaN | ||
---|---|---|---|
Product: | gcc | Reporter: | Joseph S. Myers <jsm28> |
Component: | target | Assignee: | Ramana Radhakrishnan <ramana> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | aurelien, ramana |
Priority: | P3 | Keywords: | wrong-code |
Version: | 4.9.0 | ||
Target Milestone: | 7.0 | ||
See Also: | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56828 | ||
Host: | Target: | arm*-*-* | |
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2014-02-07 00:00:00 |
Description
Joseph S. Myers
2014-01-15 19:52:07 UTC
Confirmed. *** Bug 61219 has been marked as a duplicate of this bug. *** Please note that the following patch fixes the issue: https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01421.html Need to apply Aurelien's patch - looks like that's slipped through the cracks. (In reply to Ramana Radhakrishnan from comment #4) > Need to apply Aurelien's patch - looks like that's slipped through the > cracks. What was missing to the patch was a testcase which compiles on all platform. This has happened recently, as part of the testcase for PR61441. I therefore guess this patch can now be applied. (In reply to Aurelien Jarno from comment #5) > (In reply to Ramana Radhakrishnan from comment #4) > > Need to apply Aurelien's patch - looks like that's slipped through the > > cracks. > > What was missing to the patch was a testcase which compiles on all platform. > This has happened recently, as part of the testcase for PR61441. I therefore > guess this patch can now be applied. Do you mind pinging it after another round of testing ? Thanks, Ramana (In reply to Ramana Radhakrishnan from comment #6) > (In reply to Aurelien Jarno from comment #5) > > (In reply to Ramana Radhakrishnan from comment #4) > > > Need to apply Aurelien's patch - looks like that's slipped through the > > > cracks. > > > > What was missing to the patch was a testcase which compiles on all platform. > > This has happened recently, as part of the testcase for PR61441. I therefore > > guess this patch can now be applied. > > Do you mind pinging it after another round of testing ? > I have just done a another round on testing on trunk: - The patch is still necessary. - The patch still applies, but with fuzz. I'll resend an update patch. - The patch still works as expected. - The testcase for PR61441 does not seems to work for this bug as it is compiled with -O1. GCC 6 does constant propagation and the resulting binary do not call aeabi_f2d. With GCC 5 or with -O0 the testcase also work for his bug. Is there a way to specify that a test should run with both -O0 and -O1? That would avoid writing an almost identical testcase. Aurelien On 14/07/16 12:15, aurelien at aurel32 dot net wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59833 > > --- Comment #7 from Aurelien Jarno <aurelien at aurel32 dot net> --- > (In reply to Ramana Radhakrishnan from comment #6) >> (In reply to Aurelien Jarno from comment #5) >>> (In reply to Ramana Radhakrishnan from comment #4) >>>> Need to apply Aurelien's patch - looks like that's slipped through the >>>> cracks. >>> >>> What was missing to the patch was a testcase which compiles on all platform. >>> This has happened recently, as part of the testcase for PR61441. I therefore >>> guess this patch can now be applied. >> >> Do you mind pinging it after another round of testing ? >> > > I have just done a another round on testing on trunk: > - The patch is still necessary. > - The patch still applies, but with fuzz. I'll resend an update patch. > - The patch still works as expected. > - The testcase for PR61441 does not seems to work for this bug as it is > compiled with -O1. GCC 6 does constant propagation and the resulting binary do > not call aeabi_f2d. With GCC 5 or with -O0 the testcase also work for his bug. > Is there a way to specify that a test should run with both -O0 and -O1? That > would avoid writing an almost identical testcase. An option might be to move the test into gcc.c-torture/execute and then it will torture over all optimization levels and add the option needed with dg-additional-options ? Ramana > > Aurelien > (In reply to ramana.radhakrishnan from comment #8) > On 14/07/16 12:15, aurelien at aurel32 dot net wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59833 > > > > --- Comment #7 from Aurelien Jarno <aurelien at aurel32 dot net> --- > > (In reply to Ramana Radhakrishnan from comment #6) > >> (In reply to Aurelien Jarno from comment #5) > >>> (In reply to Ramana Radhakrishnan from comment #4) > >>>> Need to apply Aurelien's patch - looks like that's slipped through the > >>>> cracks. > >>> > >>> What was missing to the patch was a testcase which compiles on all platform. > >>> This has happened recently, as part of the testcase for PR61441. I therefore > >>> guess this patch can now be applied. > >> > >> Do you mind pinging it after another round of testing ? > >> > > > > I have just done a another round on testing on trunk: > > - The patch is still necessary. > > - The patch still applies, but with fuzz. I'll resend an update patch. > > - The patch still works as expected. > > - The testcase for PR61441 does not seems to work for this bug as it is > > compiled with -O1. GCC 6 does constant propagation and the resulting binary do > > not call aeabi_f2d. With GCC 5 or with -O0 the testcase also work for his bug. > > Is there a way to specify that a test should run with both -O0 and -O1? That > > would avoid writing an almost identical testcase. > > An option might be to move the test into gcc.c-torture/execute and then it > will torture over all optimization levels and add the option needed with > dg-additional-options ? Ok, in that case, i'll add a patch specific for this bug. I am currently doing a test rebuild, I'll send a new version of the patch in the next days. Aurelien I have just posted a new version of the patch: https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01198.html Author: ramana Date: Thu Jul 21 08:27:47 2016 New Revision: 238584 URL: https://gcc.gnu.org/viewcvs?rev=238584&root=gcc&view=rev Log: [ARM] Fix PR target/59833 For Aurelien Jarno <aurelien@aurel32.net> On ARM soft-float, the float to double conversion doesn't convert a sNaN to qNaN as the IEEE Std 754 standard mandates: "Under default exception handling, any operation signaling an invalid operation exception and for which a floating-point result is to be delivered shall deliver a quiet NaN." Given the soft float ARM code ignores exceptions and always provides a result, a float to double conversion of a signaling NaN should return a quiet NaN. Fix this in extendsfdf2. gcc/ChangeLog: PR target/59833 * config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN. gcc/testsuite/ChangeLog: * gcc.dg/pr59833.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/pr59833.c Modified: trunk/libgcc/ChangeLog trunk/libgcc/config/arm/ieee754-df.S Fixed on trunk so far. This test case fails on powerpc LE as well and has been since its introduction. It isn't supported on powerpc BE. Program received signal SIGABRT, Aborted. 0x00003fffb7d00a88 in __GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #0 0x00003fffb7d00a88 in __GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00003fffb7d0686c in __GI_abort () at abort.c:89 #2 0x0000000010000748 in main () at /home/seurer/gcc/gcc-test2/gcc/testsuite/gcc.dg/pr59833.c:14 line 14 is __builtin_abort(); I was testing on trunk with revision 238762 powerpc failure of floating-point extensions to quiet signaling NaNs (because loads implicitly extend from float to double in a way that's defined as bit-manipulation rather than a convertFormat operation) is bug 56828. (In reply to joseph@codesourcery.com from comment #14) > powerpc failure of floating-point extensions to quiet signaling NaNs > (because loads implicitly extend from float to double in a way that's > defined as bit-manipulation rather than a convertFormat operation) is bug > 56828. but bug 56828 was closed as INVALID... On Fri, 16 Feb 2018, egallager at gcc dot gnu.org wrote:
> > powerpc failure of floating-point extensions to quiet signaling NaNs
> > (because loads implicitly extend from float to double in a way that's
> > defined as bit-manipulation rather than a convertFormat operation) is bug
> > 56828.
>
> but bug 56828 was closed as INVALID...
Maybe the test in that bug in fact only covered cases involving long
double (which is what that closure was for). There is an issue with
conversion from float to double on powerpc (FPRs always store values in
double format, and a load of a float value is a bitwise operation that
never raises exceptions but produces a double sNaN when loading a float
sNaN - which can be stored back to float in memory as a float sNaN with no
exceptions - meaning that converting a float value in registers to double
is treated as a no-op). It's normally OK to do that, if the converted
value will be used in double arithmetic - but if it's stored back to
memory as a double without having gone through arithmetic other than abs /
negate / copysign, it will incorrectly remain a double sNaN, and in that
case, given -fsignaling-nans, something should be done to convert it
explicitly to a qNaN (e.g. use an frsp instruction, which would do nothing
to a value representable as float which is not a signaling NaN).
Fixed for GCC-7 |