| Message ID | CAFULd4aGkGaLCth+z-aaiXm+dQY1y1zs+NU202k6X_s8tV6wFw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,i386] : Make FP inequality comparisons trapping on qNaN. | expand |
On Fri, 20 Oct 2017, Uros Bizjak wrote: > 2017-10-20 Uros Bizjak <ubizjak@gmail.com> > > * config/i386/i386.c (ix86_fp_compare_mode): Return CCFPmode > for ordered inequality comparisons even with TARGET_IEEE_FP. This is PR target/52451. A testcase (conditional on the fenv_exceptions effective-target) that ordered comparisons with quiet NaNs set FE_INVALID would be a good idea, but it would need XFAILing for powerpc (bug 58684) and s390 (bug 77918).
On Fri, Oct 20, 2017 at 2:15 PM, Joseph Myers <joseph@codesourcery.com> wrote: > This is PR target/52451. > > A testcase (conditional on the fenv_exceptions effective-target) that > ordered comparisons with quiet NaNs set FE_INVALID would be a good idea, > but it would need XFAILing for powerpc (bug 58684) and s390 (bug 77918). Joseph, thanks for pointing out a PR reference and a suggestion for a testcase. Please find attached a new version of the patch, including the comprehensive tests. 2017-10-20 Uros Bizjak <ubizjak@gmail.com> PR target/52451 * config/i386/i386.c (ix86_fp_compare_mode): Return CCFPmode for ordered inequality comparisons even with TARGET_IEEE_FP. 2017-10-20 Uros Bizjak <ubizjak@gmail.com> PR target/52451 * gcc.dg/torture/pr52451.c: New test. Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. Uros. Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 253949) +++ config/i386/i386.c (working copy) @@ -21500,14 +21500,35 @@ Return the appropriate mode to use. */ machine_mode -ix86_fp_compare_mode (enum rtx_code) +ix86_fp_compare_mode (enum rtx_code code) { - /* ??? In order to make all comparisons reversible, we do all comparisons - non-trapping when compiling for IEEE. Once gcc is able to distinguish - all forms trapping and nontrapping comparisons, we can make inequality - comparisons trapping again, since it results in better code when using - FCOM based compares. */ - return TARGET_IEEE_FP ? CCFPUmode : CCFPmode; + if (!TARGET_IEEE_FP) + return CCFPmode; + + switch (code) + { + case GT: + case GE: + case LT: + case LE: + return CCFPmode; + + case EQ: + case NE: + + case LTGT: + case UNORDERED: + case ORDERED: + case UNLT: + case UNLE: + case UNGT: + case UNGE: + case UNEQ: + return CCFPUmode; + + default: + gcc_unreachable (); + } } machine_mode Index: testsuite/gcc.dg/torture/pr52451.c =================================================================== --- testsuite/gcc.dg/torture/pr52451.c (nonexistent) +++ testsuite/gcc.dg/torture/pr52451.c (working copy) @@ -0,0 +1,55 @@ +/* { dg-do run } */ +/* { dg-add-options ieee } */ +/* { dg-require-effective-target fenv_exceptions } */ + +#include <fenv.h> + +#define TEST_C_NOEX(CMP, S) \ + r = nan##S CMP arg##S; \ + if (fetestexcept (FE_INVALID)) \ + __builtin_abort () + +#define TEST_B_NOEX(FN, S) \ + r = __builtin_##FN (nan##S, arg##S); \ + if (fetestexcept (FE_INVALID)) \ + __builtin_abort () + +#define TEST_C_EX(CMP, S) \ + r = nan##S CMP arg##S; \ + if (!fetestexcept (FE_INVALID)) \ + __builtin_abort (); \ + feclearexcept (FE_INVALID) + +#define TEST(TYPE, S) \ + volatile TYPE nan##S = __builtin_nan##S (""); \ + volatile TYPE arg##S = 1.0##S; \ + \ + TEST_C_NOEX (==, S); \ + TEST_C_NOEX (!=, S); \ + \ + TEST_B_NOEX (isgreater, S); \ + TEST_B_NOEX (isless, S); \ + TEST_B_NOEX (isgreaterequal, S); \ + TEST_B_NOEX (islessequal, S); \ + \ + TEST_B_NOEX (islessgreater, S); \ + TEST_B_NOEX (isunordered, S); \ + \ + TEST_C_EX (>, S); \ + TEST_C_EX (<, S); \ + TEST_C_EX (>=, S); \ + TEST_C_EX (<=, S) + +int +main (void) +{ + volatile int r; + + feclearexcept (FE_INVALID); + + TEST (float, f); + TEST (double, ); + TEST (long double, l); + + return 0; +}
On Fri, Oct 20, 2017 at 9:48 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Fri, Oct 20, 2017 at 2:15 PM, Joseph Myers <joseph@codesourcery.com> wrote: > >> This is PR target/52451. >> >> A testcase (conditional on the fenv_exceptions effective-target) that >> ordered comparisons with quiet NaNs set FE_INVALID would be a good idea, >> but it would need XFAILing for powerpc (bug 58684) and s390 (bug 77918). > > Joseph, > > thanks for pointing out a PR reference and a suggestion for a testcase. > > Please find attached a new version of the patch, including the > comprehensive tests. > > 2017-10-20 Uros Bizjak <ubizjak@gmail.com> > > PR target/52451 > * config/i386/i386.c (ix86_fp_compare_mode): Return CCFPmode > for ordered inequality comparisons even with TARGET_IEEE_FP. > > 2017-10-20 Uros Bizjak <ubizjak@gmail.com> > > PR target/52451 > * gcc.dg/torture/pr52451.c: New test. > > Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. Now committed to mainline SVN. Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 45a219741dbb..7ff222be9aaf 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -21500,14 +21500,35 @@ ix86_expand_int_compare (enum rtx_code code, rtx op0, rtx op1) Return the appropriate mode to use. */ machine_mode -ix86_fp_compare_mode (enum rtx_code) -{ - /* ??? In order to make all comparisons reversible, we do all comparisons - non-trapping when compiling for IEEE. Once gcc is able to distinguish - all forms trapping and nontrapping comparisons, we can make inequality - comparisons trapping again, since it results in better code when using - FCOM based compares. */ - return TARGET_IEEE_FP ? CCFPUmode : CCFPmode; +ix86_fp_compare_mode (enum rtx_code code) +{ + if (!TARGET_IEEE_FP) + return CCFPmode; + + switch (code) + { + case GT: + case GE: + case LT: + case LE: + return CCFPmode; + + case EQ: + case NE: + + case LTGT: + case UNORDERED: + case ORDERED: + case UNLT: + case UNLE: + case UNGT: + case UNGE: + case UNEQ: + return CCFPUmode; + + default: + gcc_unreachable (); + } } machine_mode
Hello! Attached patch makes FP inequality comparisons trap on qNaN. There is an old comment mentioned reversible comparisons, but middle end doesn't reverse them anymore (a couple of weeks ago, reversed comparisons were removed from i386.md): /* ??? In order to make all comparisons reversible, we do all comparisons non-trapping when compiling for IEEE. Once gcc is able to distinguish all forms trapping and nontrapping comparisons, we can make inequality comparisons trapping again, since it results in better code when using FCOM based compares. */ FCOM compares allow FP and integer memory operands. This is also what ICC produces with -fp-model strict, so I see no reason for GCC to produce different and inferior code. 2017-10-20 Uros Bizjak <ubizjak@gmail.com> * config/i386/i386.c (ix86_fp_compare_mode): Return CCFPmode for ordered inequality comparisons even with TARGET_IEEE_FP. Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. If there are no comments, I plan to commit the patch to mainline early next week. Uros.