Message ID | 20180707080605.GP7166@tucnak |
---|---|
State | New |
Headers | show |
Series | Don't fold nextafter/nexttoward if -ftrapping-math or -fmath-errno if they produce denormal results (PR c/86420) | expand |
On Sat, 7 Jul 2018, Jakub Jelinek wrote: > 2018-07-07 Jakub Jelinek <jakub@redhat.com> > > PR c/86420 > * real.c (real_nextafter): Return true if result is denormal. I have a question on the side: would it be hard / useful, in cases where nextafter may set errno or some exception flag, to fold the result to a constant while keeping the function call (ignoring the value it returns)? To clarify, I mean replace _2 = nextafter(DBL_DENORM_MIN, 0); with nextafter(DBL_DENORM_MIN, 0); _2 = 0; I think we already do that for some other calls, although I can't remember where. The point would be that we have the value of _2 and can keep folding its uses.
On Sat, Jul 07, 2018 at 11:55:17AM +0200, Marc Glisse wrote: > On Sat, 7 Jul 2018, Jakub Jelinek wrote: > > > 2018-07-07 Jakub Jelinek <jakub@redhat.com> > > > > PR c/86420 > > * real.c (real_nextafter): Return true if result is denormal. > > I have a question on the side: would it be hard / useful, in cases where > nextafter may set errno or some exception flag, to fold the result to a > constant while keeping the function call (ignoring the value it returns)? To > clarify, I mean replace > > _2 = nextafter(DBL_DENORM_MIN, 0); > > with > > nextafter(DBL_DENORM_MIN, 0); > _2 = 0; > > I think we already do that for some other calls, although I can't remember > where. The point would be that we have the value of _2 and can keep folding > its uses. For errno purposes alone that would be possible, but the function is marked #define ATTR_MATHFN_ERRNO (flag_errno_math ? \ ATTR_NOTHROW_LEAF_LIST : ATTR_CONST_NOTHROW_LEAF_LIST) and thus with -ftrapping-math -fno-math-errno I'm afraid we'd immediately DCE the call in the second form (without lhs). Jakub
On Sat, 7 Jul 2018, Jakub Jelinek wrote: > On Sat, Jul 07, 2018 at 11:55:17AM +0200, Marc Glisse wrote: >> On Sat, 7 Jul 2018, Jakub Jelinek wrote: >> >>> 2018-07-07 Jakub Jelinek <jakub@redhat.com> >>> >>> PR c/86420 >>> * real.c (real_nextafter): Return true if result is denormal. >> >> I have a question on the side: would it be hard / useful, in cases where >> nextafter may set errno or some exception flag, to fold the result to a >> constant while keeping the function call (ignoring the value it returns)? To >> clarify, I mean replace >> >> _2 = nextafter(DBL_DENORM_MIN, 0); >> >> with >> >> nextafter(DBL_DENORM_MIN, 0); >> _2 = 0; >> >> I think we already do that for some other calls, although I can't remember >> where. The point would be that we have the value of _2 and can keep folding >> its uses. > > For errno purposes alone that would be possible, but the function is marked > #define ATTR_MATHFN_ERRNO (flag_errno_math ? \ > ATTR_NOTHROW_LEAF_LIST : ATTR_CONST_NOTHROW_LEAF_LIST) > and thus with -ftrapping-math -fno-math-errno I'm afraid we'd immediately > DCE the call in the second form (without lhs). That looks like a problem we'll have to fix eventually. But not as part of this patch indeed.
On Sat, 7 Jul 2018, Jakub Jelinek wrote: > Hi! > > So, apparently I've misread when exceptions are raised by > nextafter/nexttoward (and errno set). > if(((ix>=0x7ff00000)&&((ix-0x7ff00000)|lx)!=0) || /* x is nan */ > ((iy>=0x7ff00000)&&((iy-0x7ff00000)|ly)!=0)) /* y is nan */ > return x+y; > I believe the above only raises exception if either operand is sNaN and thus > we should handle it: > if (REAL_VALUE_ISSIGNALING_NAN (*arg0) > || REAL_VALUE_ISSIGNALING_NAN (*arg1)) > return false; > Then: > if(x==y) return y; /* x=y, return y */ > If arguments are equal, no exception is raised even if it is denormal, > we handle this with: > /* If x == y, return y cast to target type. */ > if (cmp == 0) > { > real_convert (r, fmt, y); > return false; > } > Next: > if((ix|lx)==0) { /* x == 0 */ > double u; > INSERT_WORDS(x,hy&0x80000000,1); /* return +-minsubnormal */ > u = math_opt_barrier (x); > u = u*u; > math_force_eval (u); /* raise underflow flag */ > return x; > } > going from zero to +/- ulp should only raise underflow, but not set errno, > handled with: > /* Similarly for nextafter (0, 1) raising underflow. */ > else if (flag_trapping_math > && arg0->cl == rvc_zero > && result->cl != rvc_zero) > return false; > Then overflow: > hy = hx&0x7ff00000; > if(hy>=0x7ff00000) { > double u = x+x; /* overflow */ > math_force_eval (u); > __set_errno (ERANGE); > } > should be handled with: > if (REAL_EXP (r) > fmt->emax) > { > get_inf (r, x->sign); > return true; > } > And finally there is: > if(hy<0x00100000) { > double u = x*x; /* underflow */ > math_force_eval (u); /* raise underflow flag */ > __set_errno (ERANGE); > } > which I've misread as raising exception and setting errno only if > the result is 0 and first arg isn't: > return r->cl == rvc_zero; > but actually it is true also for all denormal returns except for the x == y > case, so the following patch uses: > return r->cl == rvc_zero || REAL_EXP (r) < fmt->emin; > instead. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Thanks, Richard. > 2018-07-07 Jakub Jelinek <jakub@redhat.com> > > PR c/86420 > * real.c (real_nextafter): Return true if result is denormal. > > * gcc.dg/nextafter-1.c (TEST): Adjust the tests that expect denormals > to be returned and when first argument is not 0, so that they don't do > anything for NEED_EXC or NEED_ERRNO. > > --- gcc/real.c.jj 2018-05-06 23:12:49.211619736 +0200 > +++ gcc/real.c 2018-07-06 18:42:44.761026632 +0200 > @@ -5141,7 +5141,7 @@ real_nextafter (REAL_VALUE_TYPE *r, form > get_zero (r, x->sign); > return true; > } > - return r->cl == rvc_zero; > + return r->cl == rvc_zero || REAL_EXP (r) < fmt->emin; > } > > /* Write into BUF the maximum representable finite floating-point > --- gcc/testsuite/gcc.dg/nextafter-1.c.jj 2018-05-10 09:38:03.040250709 +0200 > +++ gcc/testsuite/gcc.dg/nextafter-1.c 2018-07-06 19:17:55.138355524 +0200 > @@ -58,23 +58,41 @@ name (void) \ > = (NEED_EXC || NEED_ERRNO) ? __builtin_inf##l1 () \ > : fn (MAX1, __builtin_inf ()); \ > CHECK (__builtin_isinf##l1 (m) && !__builtin_signbit (m)); \ > - const type n = fn (DENORM_MIN1, 12.0##L2); \ > + const type n \ > + = (NEED_EXC || NEED_ERRNO) ? 2.0##L1 * DENORM_MIN1 \ > + : fn (DENORM_MIN1, 12.0##L2); \ > CHECK (n == 2.0##L1 * DENORM_MIN1); \ > - const type o = fn (n, 24.0##L2); \ > + const type o \ > + = (NEED_EXC || NEED_ERRNO) ? 3.0##L1 * DENORM_MIN1 \ > + : fn (n, 24.0##L2); \ > CHECK (o == 3.0##L1 * DENORM_MIN1); \ > - const type p = fn (o, 132.0##L2); \ > + const type p \ > + = (NEED_EXC || NEED_ERRNO) ? 4.0##L1 * DENORM_MIN1 \ > + : fn (o, 132.0##L2); \ > CHECK (p == 4.0##L1 * DENORM_MIN1); \ > - const type q = fn (2.0##L1 * DENORM_MIN1, -__builtin_inf ()); \ > + const type q \ > + = (NEED_EXC || NEED_ERRNO) ? DENORM_MIN1 \ > + : fn (2.0##L1 * DENORM_MIN1, -__builtin_inf ()); \ > CHECK (q == DENORM_MIN1); \ > - const type r = fn (3.0##L1 * DENORM_MIN1, DENORM_MIN2); \ > + const type r \ > + = (NEED_EXC || NEED_ERRNO) ? 2.0##L1 * DENORM_MIN1 \ > + : fn (3.0##L1 * DENORM_MIN1, DENORM_MIN2); \ > CHECK (r == 2.0##L1 * DENORM_MIN1); \ > - const type s = fn (4.0##L1 * DENORM_MIN1, 2.0##L2 * DENORM_MIN2); \ > + const type s \ > + = (NEED_EXC || NEED_ERRNO) ? 3.0##L1 * DENORM_MIN1 \ > + : fn (4.0##L1 * DENORM_MIN1, 2.0##L2 * DENORM_MIN2); \ > CHECK (s == 3.0##L1 * DENORM_MIN1); \ > - const type t = fn (MIN1, 0.0##L2); \ > + const type t \ > + = (NEED_EXC || NEED_ERRNO) ? MIN1 - DENORM_MIN1 \ > + : fn (MIN1, 0.0##L2); \ > CHECK (t == MIN1 - DENORM_MIN1); \ > - const type u = fn (MIN1 - DENORM_MIN1, -MIN2); \ > + const type u \ > + = (NEED_EXC || NEED_ERRNO) ? MIN1 - 2.0##L1 * DENORM_MIN1 \ > + : fn (MIN1 - DENORM_MIN1, -MIN2); \ > CHECK (u == MIN1 - 2.0##L1 * DENORM_MIN1); \ > - const type v = fn (MIN1 - 2.0##L1 * DENORM_MIN1, 100.0##L2); \ > + const type v \ > + = (NEED_EXC || NEED_ERRNO) ? MIN1 - DENORM_MIN1 \ > + : fn (MIN1 - 2.0##L1 * DENORM_MIN1, 100.0##L2); \ > CHECK (v == MIN1 - DENORM_MIN1); \ > const type w = fn (MIN1 - DENORM_MIN1, MAX2); \ > CHECK (w == MIN1); \ > @@ -82,9 +100,13 @@ name (void) \ > CHECK (x == MIN1 + DENORM_MIN1); \ > const type y = fn (MIN1 + DENORM_MIN1, __builtin_inf##l2 ()); \ > CHECK (y == MIN1 + 2.0##L1 * DENORM_MIN1); \ > - const type z = fn (MIN1 / 2.0##L1, -MIN2); \ > + const type z \ > + = (NEED_EXC || NEED_ERRNO) ? MIN1 / 2.0##L1 - DENORM_MIN1 \ > + : fn (MIN1 / 2.0##L1, -MIN2); \ > CHECK (z == MIN1 / 2.0##L1 - DENORM_MIN1); \ > - const type aa = fn (-MIN1 / 4.0##L1, MIN2); \ > + const type aa \ > + = (NEED_EXC || NEED_ERRNO) ? -MIN1 / 4.0##L1 + DENORM_MIN1 \ > + : fn (-MIN1 / 4.0##L1, MIN2); \ > CHECK (aa == -MIN1 / 4.0##L1 + DENORM_MIN1); \ > const type ab = fn (MIN1 * 2.0##L1, -MIN2); \ > CHECK (ab == MIN1 * 2.0##L1 - DENORM_MIN1); \ > @@ -92,9 +114,13 @@ name (void) \ > CHECK (ac == MIN1 * 4.0##L1 - DENORM_MIN1 * 2.0##L1); \ > const type ad = fn (MIN1 * 64.0##L1, MIN2); \ > CHECK (ad == MIN1 * 64.0##L1 - DENORM_MIN1 * 32.0##L1); \ > - const type ae = fn (MIN1 / 2.0##L1 - DENORM_MIN1, 100.0##L2); \ > + const type ae \ > + = (NEED_EXC || NEED_ERRNO) ? MIN1 / 2.0##L1 \ > + : fn (MIN1 / 2.0##L1 - DENORM_MIN1, 100.0##L2); \ > CHECK (ae == MIN1 / 2.0##L1); \ > - const type af = fn (-MIN1 / 4 + DENORM_MIN1, -100.0##L2); \ > + const type af \ > + = (NEED_EXC || NEED_ERRNO) ? -MIN1 / 4.0##L1 \ > + : fn (-MIN1 / 4 + DENORM_MIN1, -100.0##L2); \ > CHECK (af == -MIN1 / 4.0##L1); \ > const type ag = fn (MIN1 * 2.0##L1 - DENORM_MIN1, 100.0##L2); \ > CHECK (ag == MIN1 * 2.0##L1); \ > > Jakub > >
On Sat, 7 Jul 2018, Marc Glisse wrote: > On Sat, 7 Jul 2018, Jakub Jelinek wrote: > > > 2018-07-07 Jakub Jelinek <jakub@redhat.com> > > > > PR c/86420 > > * real.c (real_nextafter): Return true if result is denormal. > > I have a question on the side: would it be hard / useful, in cases where > nextafter may set errno or some exception flag, to fold the result to a > constant while keeping the function call (ignoring the value it returns)? To > clarify, I mean replace > > _2 = nextafter(DBL_DENORM_MIN, 0); > > with > > nextafter(DBL_DENORM_MIN, 0); > _2 = 0; > > I think we already do that for some other calls, although I can't remember > where. The point would be that we have the value of _2 and can keep folding > its uses. There's tree-call-dce.c which is doing a related (more complex) transform but nothing doing constant propagation through calls. I think it would be more useful for the target C library to expose sth like set_errno (...) so we can constant fold the errno setting as well. Maybe there's some cheap non-DCEable math function we could abuse... Richard.
--- gcc/real.c.jj 2018-05-06 23:12:49.211619736 +0200 +++ gcc/real.c 2018-07-06 18:42:44.761026632 +0200 @@ -5141,7 +5141,7 @@ real_nextafter (REAL_VALUE_TYPE *r, form get_zero (r, x->sign); return true; } - return r->cl == rvc_zero; + return r->cl == rvc_zero || REAL_EXP (r) < fmt->emin; } /* Write into BUF the maximum representable finite floating-point --- gcc/testsuite/gcc.dg/nextafter-1.c.jj 2018-05-10 09:38:03.040250709 +0200 +++ gcc/testsuite/gcc.dg/nextafter-1.c 2018-07-06 19:17:55.138355524 +0200 @@ -58,23 +58,41 @@ name (void) \ = (NEED_EXC || NEED_ERRNO) ? __builtin_inf##l1 () \ : fn (MAX1, __builtin_inf ()); \ CHECK (__builtin_isinf##l1 (m) && !__builtin_signbit (m)); \ - const type n = fn (DENORM_MIN1, 12.0##L2); \ + const type n \ + = (NEED_EXC || NEED_ERRNO) ? 2.0##L1 * DENORM_MIN1 \ + : fn (DENORM_MIN1, 12.0##L2); \ CHECK (n == 2.0##L1 * DENORM_MIN1); \ - const type o = fn (n, 24.0##L2); \ + const type o \ + = (NEED_EXC || NEED_ERRNO) ? 3.0##L1 * DENORM_MIN1 \ + : fn (n, 24.0##L2); \ CHECK (o == 3.0##L1 * DENORM_MIN1); \ - const type p = fn (o, 132.0##L2); \ + const type p \ + = (NEED_EXC || NEED_ERRNO) ? 4.0##L1 * DENORM_MIN1 \ + : fn (o, 132.0##L2); \ CHECK (p == 4.0##L1 * DENORM_MIN1); \ - const type q = fn (2.0##L1 * DENORM_MIN1, -__builtin_inf ()); \ + const type q \ + = (NEED_EXC || NEED_ERRNO) ? DENORM_MIN1 \ + : fn (2.0##L1 * DENORM_MIN1, -__builtin_inf ()); \ CHECK (q == DENORM_MIN1); \ - const type r = fn (3.0##L1 * DENORM_MIN1, DENORM_MIN2); \ + const type r \ + = (NEED_EXC || NEED_ERRNO) ? 2.0##L1 * DENORM_MIN1 \ + : fn (3.0##L1 * DENORM_MIN1, DENORM_MIN2); \ CHECK (r == 2.0##L1 * DENORM_MIN1); \ - const type s = fn (4.0##L1 * DENORM_MIN1, 2.0##L2 * DENORM_MIN2); \ + const type s \ + = (NEED_EXC || NEED_ERRNO) ? 3.0##L1 * DENORM_MIN1 \ + : fn (4.0##L1 * DENORM_MIN1, 2.0##L2 * DENORM_MIN2); \ CHECK (s == 3.0##L1 * DENORM_MIN1); \ - const type t = fn (MIN1, 0.0##L2); \ + const type t \ + = (NEED_EXC || NEED_ERRNO) ? MIN1 - DENORM_MIN1 \ + : fn (MIN1, 0.0##L2); \ CHECK (t == MIN1 - DENORM_MIN1); \ - const type u = fn (MIN1 - DENORM_MIN1, -MIN2); \ + const type u \ + = (NEED_EXC || NEED_ERRNO) ? MIN1 - 2.0##L1 * DENORM_MIN1 \ + : fn (MIN1 - DENORM_MIN1, -MIN2); \ CHECK (u == MIN1 - 2.0##L1 * DENORM_MIN1); \ - const type v = fn (MIN1 - 2.0##L1 * DENORM_MIN1, 100.0##L2); \ + const type v \ + = (NEED_EXC || NEED_ERRNO) ? MIN1 - DENORM_MIN1 \ + : fn (MIN1 - 2.0##L1 * DENORM_MIN1, 100.0##L2); \ CHECK (v == MIN1 - DENORM_MIN1); \ const type w = fn (MIN1 - DENORM_MIN1, MAX2); \ CHECK (w == MIN1); \ @@ -82,9 +100,13 @@ name (void) \ CHECK (x == MIN1 + DENORM_MIN1); \ const type y = fn (MIN1 + DENORM_MIN1, __builtin_inf##l2 ()); \ CHECK (y == MIN1 + 2.0##L1 * DENORM_MIN1); \ - const type z = fn (MIN1 / 2.0##L1, -MIN2); \ + const type z \ + = (NEED_EXC || NEED_ERRNO) ? MIN1 / 2.0##L1 - DENORM_MIN1 \ + : fn (MIN1 / 2.0##L1, -MIN2); \ CHECK (z == MIN1 / 2.0##L1 - DENORM_MIN1); \ - const type aa = fn (-MIN1 / 4.0##L1, MIN2); \ + const type aa \ + = (NEED_EXC || NEED_ERRNO) ? -MIN1 / 4.0##L1 + DENORM_MIN1 \ + : fn (-MIN1 / 4.0##L1, MIN2); \ CHECK (aa == -MIN1 / 4.0##L1 + DENORM_MIN1); \ const type ab = fn (MIN1 * 2.0##L1, -MIN2); \ CHECK (ab == MIN1 * 2.0##L1 - DENORM_MIN1); \ @@ -92,9 +114,13 @@ name (void) \ CHECK (ac == MIN1 * 4.0##L1 - DENORM_MIN1 * 2.0##L1); \ const type ad = fn (MIN1 * 64.0##L1, MIN2); \ CHECK (ad == MIN1 * 64.0##L1 - DENORM_MIN1 * 32.0##L1); \ - const type ae = fn (MIN1 / 2.0##L1 - DENORM_MIN1, 100.0##L2); \ + const type ae \ + = (NEED_EXC || NEED_ERRNO) ? MIN1 / 2.0##L1 \ + : fn (MIN1 / 2.0##L1 - DENORM_MIN1, 100.0##L2); \ CHECK (ae == MIN1 / 2.0##L1); \ - const type af = fn (-MIN1 / 4 + DENORM_MIN1, -100.0##L2); \ + const type af \ + = (NEED_EXC || NEED_ERRNO) ? -MIN1 / 4.0##L1 \ + : fn (-MIN1 / 4 + DENORM_MIN1, -100.0##L2); \ CHECK (af == -MIN1 / 4.0##L1); \ const type ag = fn (MIN1 * 2.0##L1 - DENORM_MIN1, 100.0##L2); \ CHECK (ag == MIN1 * 2.0##L1); \