| Message ID | alpine.LSU.2.20.1902191343260.23386@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
| Series | Fix PR88074 | expand |
On 19/02/2019 12:45, Richard Biener wrote: > > The following limits mpfr operations to the maximum exponent range as > determined by available float modes. This avoids excessive work > for evaluating functions like ctan for large arguments. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. > > Richard. > > 2019-02-19 Richard Biener <rguenther@suse.de> > > PR middle-end/88074 > * toplev.c (do_compile): Initialize mpfr's exponent range > based on available float modes. > > * gcc.dg/pr88074.c: New testcase. This patch causes a number of test regressions for (at least) arm and amdgcn. I've run a bisect and confirmed this is the first commit where it doesn't work. spawn -ignore SIGHUP amdgcn-unknown-amdhsa-gcc .../gcc/testsuite/gcc.dg/torture/builtin-math-7.c -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -O0 -lm -o ./builtin-math-7.exe^M ld: error: undefined symbol: link_error^M >>> referenced by /tmp/ccKynVNn.o:(main)^M ^M ld: error: undefined symbol: link_error^M >>> referenced by /tmp/ccKynVNn.o:(main)^M collect2: error: ld returned 1 exit status^M compiler exited with status 1 output is: ld: error: undefined symbol: link_error^M >>> referenced by /tmp/ccKynVNn.o:(main)^M ^M ld: error: undefined symbol: link_error^M >>> referenced by /tmp/ccKynVNn.o:(main)^M collect2: error: ld returned 1 exit status^M FAIL: gcc.dg/torture/builtin-math-7.c -O0 (test for excess errors) I see the same failures posted here: https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg02389.html Andrew
On Wed, Feb 20, 2019 at 11:13:54AM +0000, Andrew Stubbs wrote: > This patch causes a number of test regressions for (at least) arm and > amdgcn. I've run a bisect and confirmed this is the first commit where it > doesn't work. The following, so far untested (except for x86_64->armv7hl-linux-gnueabi cross on the builtin-math-7.c testcase and x86_64-linux native on the pr88074* testcases) patch should cure this, will bootstrap/regtest it tonight. 2019-02-20 Jakub Jelinek <jakub@redhat.com> PR middle-end/88074 * toplev.c (do_compile): Double the emin/emax exponents to workaround buggy mpc_norm. * gcc.dg/pr88074-2.c: New test. --- gcc/toplev.c.jj 2019-02-20 10:00:49.290492694 +0100 +++ gcc/toplev.c 2019-02-20 14:04:27.037769362 +0100 @@ -2173,8 +2173,12 @@ do_compile () max_exp = fmt->emax; } } - if (mpfr_set_emin (min_exp) - || mpfr_set_emax (max_exp)) + /* mpc_norm assumes it can square a number without bothering with + with range scaling, so until that is fixed, double the minimum + and maximum exponents, plus add some buffer for arithmetics + on the squared numbers. */ + if (mpfr_set_emin (2 * (min_exp - 1)) + || mpfr_set_emax (2 * (max_exp + 1))) sorry ("mpfr not configured to handle all float modes"); /* Set up the back-end if requested. */ --- gcc/testsuite/gcc.dg/pr88074-2.c.jj 2019-02-20 14:10:07.109140297 +0100 +++ gcc/testsuite/gcc.dg/pr88074-2.c 2019-02-20 14:09:46.696478179 +0100 @@ -0,0 +1,17 @@ +/* PR middle-end/88074 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-add-options float128 } */ +/* { dg-require-effective-target float128 } */ +/* { dg-final { scan-tree-dump-not "link_error " "optimized" } } */ + +extern void link_error (void); +int +main () +{ + if (((__FLT128_MAX__ * 0.5 + __FLT128_MAX__ * 0.5i) + / (__FLT128_MAX__ * 0.25 + __FLT128_MAX__ * 0.25i)) + != (_Complex _Float128) 2) + link_error (); + return 0; +} Jakub
On Wed, 20 Feb 2019 at 12:14, Andrew Stubbs <ams@codesourcery.com> wrote: > > On 19/02/2019 12:45, Richard Biener wrote: > > > > The following limits mpfr operations to the maximum exponent range as > > determined by available float modes. This avoids excessive work > > for evaluating functions like ctan for large arguments. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. > > > > Richard. > > > > 2019-02-19 Richard Biener <rguenther@suse.de> > > > > PR middle-end/88074 > > * toplev.c (do_compile): Initialize mpfr's exponent range > > based on available float modes. > > > > * gcc.dg/pr88074.c: New testcase. > > This patch causes a number of test regressions for (at least) arm and > amdgcn. I've run a bisect and confirmed this is the first commit where > it doesn't work. > > spawn -ignore SIGHUP amdgcn-unknown-amdhsa-gcc > .../gcc/testsuite/gcc.dg/torture/builtin-math-7.c > -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers > -fdiagnostics-color=never -O0 -lm -o ./builtin-math-7.exe^M > ld: error: undefined symbol: link_error^M > >>> referenced by /tmp/ccKynVNn.o:(main)^M > ^M > ld: error: undefined symbol: link_error^M > >>> referenced by /tmp/ccKynVNn.o:(main)^M > collect2: error: ld returned 1 exit status^M > compiler exited with status 1 > output is: > ld: error: undefined symbol: link_error^M > >>> referenced by /tmp/ccKynVNn.o:(main)^M > ^M > ld: error: undefined symbol: link_error^M > >>> referenced by /tmp/ccKynVNn.o:(main)^M > collect2: error: ld returned 1 exit status^M > > FAIL: gcc.dg/torture/builtin-math-7.c -O0 (test for excess errors) > > I see the same failures posted here: > https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg02389.html > See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89415 > Andrew
On 2/20/19 6:22 AM, Jakub Jelinek wrote: > On Wed, Feb 20, 2019 at 11:13:54AM +0000, Andrew Stubbs wrote: >> This patch causes a number of test regressions for (at least) arm and >> amdgcn. I've run a bisect and confirmed this is the first commit where it >> doesn't work. > > The following, so far untested (except for x86_64->armv7hl-linux-gnueabi > cross on the builtin-math-7.c testcase and x86_64-linux native on the > pr88074* testcases) patch should cure this, will bootstrap/regtest it > tonight. > > 2019-02-20 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/88074 > * toplev.c (do_compile): Double the emin/emax exponents to workaround > buggy mpc_norm. > > * gcc.dg/pr88074-2.c: New test. I've got a couple dozen crosses failing because of the recent changes. I can throw this into the tester and check all of them pretty easily if that would be helpful. jeff
On Wed, Feb 20, 2019 at 06:45:51AM -0700, Jeff Law wrote: > On 2/20/19 6:22 AM, Jakub Jelinek wrote: > > On Wed, Feb 20, 2019 at 11:13:54AM +0000, Andrew Stubbs wrote: > >> This patch causes a number of test regressions for (at least) arm and > >> amdgcn. I've run a bisect and confirmed this is the first commit where it > >> doesn't work. > > > > The following, so far untested (except for x86_64->armv7hl-linux-gnueabi > > cross on the builtin-math-7.c testcase and x86_64-linux native on the > > pr88074* testcases) patch should cure this, will bootstrap/regtest it > > tonight. > > > > 2019-02-20 Jakub Jelinek <jakub@redhat.com> > > > > PR middle-end/88074 > > * toplev.c (do_compile): Double the emin/emax exponents to workaround > > buggy mpc_norm. > > > > * gcc.dg/pr88074-2.c: New test. > I've got a couple dozen crosses failing because of the recent changes. > I can throw this into the tester and check all of them pretty easily if > that would be helpful. Yes, it would be helpful. I've just checked the aarch64 sinatan-1.c and the patch restores the previous behavior on that testcase too. Jakub
On Wed, 20 Feb 2019 at 14:51, Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Feb 20, 2019 at 06:45:51AM -0700, Jeff Law wrote: > > On 2/20/19 6:22 AM, Jakub Jelinek wrote: > > > On Wed, Feb 20, 2019 at 11:13:54AM +0000, Andrew Stubbs wrote: > > >> This patch causes a number of test regressions for (at least) arm and > > >> amdgcn. I've run a bisect and confirmed this is the first commit where it > > >> doesn't work. > > > > > > The following, so far untested (except for x86_64->armv7hl-linux-gnueabi > > > cross on the builtin-math-7.c testcase and x86_64-linux native on the > > > pr88074* testcases) patch should cure this, will bootstrap/regtest it > > > tonight. > > > > > > 2019-02-20 Jakub Jelinek <jakub@redhat.com> > > > > > > PR middle-end/88074 > > > * toplev.c (do_compile): Double the emin/emax exponents to workaround > > > buggy mpc_norm. > > > > > > * gcc.dg/pr88074-2.c: New test. > > I've got a couple dozen crosses failing because of the recent changes. > > I can throw this into the tester and check all of them pretty easily if > > that would be helpful. > > Yes, it would be helpful. I've just checked the aarch64 sinatan-1.c and the > patch restores the previous behavior on that testcase too. > FWIW, I've sent your patch for testing too (cross arm and aarch64, c/c++/fortran) Christophe > Jakub
On 20/02/2019 13:22, Jakub Jelinek wrote: > On Wed, Feb 20, 2019 at 11:13:54AM +0000, Andrew Stubbs wrote: >> This patch causes a number of test regressions for (at least) arm and >> amdgcn. I've run a bisect and confirmed this is the first commit where it >> doesn't work. > > The following, so far untested (except for x86_64->armv7hl-linux-gnueabi > cross on the builtin-math-7.c testcase and x86_64-linux native on the > pr88074* testcases) patch should cure this, will bootstrap/regtest it > tonight. I've confirmed that it fixes this specific testcase on amdgcn. Andrew
On 2/20/19 6:51 AM, Jakub Jelinek wrote: > On Wed, Feb 20, 2019 at 06:45:51AM -0700, Jeff Law wrote: >> On 2/20/19 6:22 AM, Jakub Jelinek wrote: >>> On Wed, Feb 20, 2019 at 11:13:54AM +0000, Andrew Stubbs wrote: >>>> This patch causes a number of test regressions for (at least) arm and >>>> amdgcn. I've run a bisect and confirmed this is the first commit where it >>>> doesn't work. >>> >>> The following, so far untested (except for x86_64->armv7hl-linux-gnueabi >>> cross on the builtin-math-7.c testcase and x86_64-linux native on the >>> pr88074* testcases) patch should cure this, will bootstrap/regtest it >>> tonight. >>> >>> 2019-02-20 Jakub Jelinek <jakub@redhat.com> >>> >>> PR middle-end/88074 >>> * toplev.c (do_compile): Double the emin/emax exponents to workaround >>> buggy mpc_norm. >>> >>> * gcc.dg/pr88074-2.c: New test. >> I've got a couple dozen crosses failing because of the recent changes. >> I can throw this into the tester and check all of them pretty easily if >> that would be helpful. > > Yes, it would be helpful. I've just checked the aarch64 sinatan-1.c and the > patch restores the previous behavior on that testcase too. Good, then I won't watch sparc64 that closely. It takes *hours* to run and it flagged sinatan-1 as well. Figure I'll have results for all the *-elf thingies in my tester in an hour or so. jeff
On 2/20/19 6:17 AM, Jakub Jelinek wrote: > On Wed, Feb 20, 2019 at 11:13:54AM +0000, Andrew Stubbs wrote: >> This patch causes a number of test regressions for (at least) arm and >> amdgcn. I've run a bisect and confirmed this is the first commit where it >> doesn't work. > > The following, so far untested (except for x86_64->armv7hl-linux-gnueabi > cross on the builtin-math-7.c testcase and x86_64-linux native on the > pr88074* testcases) patch should cure this, will bootstrap/regtest it > tonight. > > 2019-02-20 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/88074 > * toplev.c (do_compile): Double the emin/emax exponents to workaround > buggy mpc_norm. > > * gcc.dg/pr88074-2.c: New test. That fixed all the *-elf regressions in my tester (arc, nds32be, v850e, mn10300, msp430, nds32le, lm3, visium, or1k, cris, iq2000, fr30, m32r, mcore, crisv32 all flagged it as a regression). I suspect it'll fix the regressions on with sparc64, aarch64 & s390x native linux since they all regressed just the sinatan test. The powerpc64 and powerpc64le platforms continue to have all kinds of problems. The amount of instability from day to day is concerning. jeff
On Wed, 20 Feb 2019 at 15:59, Jeff Law <law@redhat.com> wrote: > > On 2/20/19 6:17 AM, Jakub Jelinek wrote: > > On Wed, Feb 20, 2019 at 11:13:54AM +0000, Andrew Stubbs wrote: > >> This patch causes a number of test regressions for (at least) arm and > >> amdgcn. I've run a bisect and confirmed this is the first commit where it > >> doesn't work. > > > > The following, so far untested (except for x86_64->armv7hl-linux-gnueabi > > cross on the builtin-math-7.c testcase and x86_64-linux native on the > > pr88074* testcases) patch should cure this, will bootstrap/regtest it > > tonight. > > > > 2019-02-20 Jakub Jelinek <jakub@redhat.com> > > > > PR middle-end/88074 > > * toplev.c (do_compile): Double the emin/emax exponents to workaround > > buggy mpc_norm. > > > > * gcc.dg/pr88074-2.c: New test. > That fixed all the *-elf regressions in my tester (arc, nds32be, v850e, > mn10300, msp430, nds32le, lm3, visium, or1k, cris, iq2000, fr30, m32r, > mcore, crisv32 all flagged it as a regression). > > I suspect it'll fix the regressions on with sparc64, aarch64 & s390x > native linux since they all regressed just the sinatan test. > > The powerpc64 and powerpc64le platforms continue to have all kinds of > problems. The amount of instability from day to day is concerning. > Jakub's patch does fix the regressions I reported on aarch64 and arm (sinatan, builtin-math-7 and the fortran one) Thanks > jeff
On Wed, 20 Feb 2019, Jakub Jelinek wrote: > + /* mpc_norm assumes it can square a number without bothering with > + with range scaling, so until that is fixed, double the minimum > + and maximum exponents, plus add some buffer for arithmetics > + on the squared numbers. */ Note that I think such issues are pervasive in MPC; it's not just that one function. MPFR is generally careful about using a larger exponent range internally (there might still be issues with very big exponents where they aren't handled specially to avoid internal overflow, but the maximum MPFR exponent range is very large); MPC doesn't generally try to handle restricted exponent ranges.
diff --git a/gcc/testsuite/gcc.dg/pr88074.c b/gcc/testsuite/gcc.dg/pr88074.c new file mode 100644 index 00000000000..9f64cc11424 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr88074.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O" } */ + +#include <complex.h> + +int main() +{ + _Complex double x; + __real x = 3.091e+8; + __imag x = -4.045e+8; + /* This used to spend huge amounts of compile-time inside mpc. */ + volatile _Complex double y = ctan (x); + return 0; +} diff --git a/gcc/toplev.c b/gcc/toplev.c index ab20cd98969..fa8e1b78c99 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -2153,6 +2153,30 @@ do_compile () else int_n_enabled_p[i] = false; + /* Initialize mpfrs exponent range. This is important to get + underflow/overflow in a reasonable timeframe. */ + machine_mode mode; + int min_exp = -1; + int max_exp = 1; + FOR_EACH_MODE_IN_CLASS (mode, MODE_FLOAT) + if (SCALAR_FLOAT_MODE_P (mode)) + { + const real_format *fmt = REAL_MODE_FORMAT (mode); + if (fmt) + { + /* fmt->emin - fmt->p + 1 should be enough but the + back-and-forth dance in real_to_decimal_for_mode we + do for checking fails due to rounding effects then. */ + if ((fmt->emin - fmt->p) < min_exp) + min_exp = fmt->emin - fmt->p; + if (fmt->emax > max_exp) + max_exp = fmt->emax; + } + } + if (mpfr_set_emin (min_exp) + || mpfr_set_emax (max_exp)) + sorry ("mpfr not configured to handle all float modes"); + /* Set up the back-end if requested. */ if (!no_backend) backend_init ();
The following limits mpfr operations to the maximum exponent range as determined by available float modes. This avoids excessive work for evaluating functions like ctan for large arguments. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2019-02-19 Richard Biener <rguenther@suse.de> PR middle-end/88074 * toplev.c (do_compile): Initialize mpfr's exponent range based on available float modes. * gcc.dg/pr88074.c: New testcase.