| Message ID | Pine.LNX.4.64.1409191817180.11496@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
Ping. This patch <https://sourceware.org/ml/libc-alpha/2014-09/msg00488.html> is pending review.
Ping^2. This patch <https://sourceware.org/ml/libc-alpha/2014-09/msg00488.html> is pending review.
On 09/19/2014 02:18 PM, Joseph S. Myers wrote: > Continuing the addition of soft-fp features used in the Linux kernel, > this patch adds soft-fp support for FP_DENORM_ZERO (flushing input > subnormal operands to zero of the same sign). (This patch is relative > to a tree with > <https://sourceware.org/ml/libc-alpha/2014-09/msg00411.html>, > <https://sourceware.org/ml/libc-alpha/2014-09/msg00442.html>, > <https://sourceware.org/ml/libc-alpha/2014-09/msg00461.html>, > <https://sourceware.org/ml/libc-alpha/2014-09/msg00463.html> and > <https://sourceware.org/ml/libc-alpha/2014-09/msg00464.html> applied, > for convenience rather than because of any essential dependency on the > other patches.) > > There are some differences from the kernel version. In the kernel, > the "inexact" exception is set when flushing to zero. This does not > appear to match the documented semantics for either of the > architectures (alpha and sh) for which the kernel uses FP_DENORM_ZERO, > so this patch does not set "inexact" in this case. More operations > now use raw or semi-raw unpacking for optimization than did in the > ten-year-old soft-fp version in the kernel, so checks of > FP_DENORM_ZERO are inserted in those operations. They are also > inserted for comparisons (which already used raw unpacking in the old > version) as I believe that's the correct thing to do when input > subnormals are flushed to zero. They are *not* inserted for _FP_NEG. > (If any processors do flush input subnormals to zero for negation, or > otherwise vary from the rules implemented when FP_DENORM_ZERO is set, > further macros for sfp-machine.h to control this may need to be > added.) > > Although the addition for comparisons will cause FP_EX_DENORM to be > set in this case, it still won't be set for comparisons involving > subnormals when not flushed to zero. It's quite possible that > accurate emulation of processors that have such an exception for > subnormal operands will require further changes relating to when > FP_EX_DENORM is set (in general, the support for things defined by > IEEE should be considered more reliable and mature than the support > for things outside the scope of IEEE floating point). > > Although some processors also have a mode for abrupt underflow - > producing zeroes instead of output subnormals - there is no such mode > in the kernel's soft-fp, so no such mode is added to glibc's soft-fp > (although it could be if someone wanted to emulate such processor > support). > > Tested for powerpc-nofpu that the disassembly of installed shared > libraries is unchanged by this patch. > > 2014-09-19 Joseph Myers <joseph@codesourcery.com> > > * soft-fp/soft-fp.h (FP_DENORM_ZERO): New macro. > * soft-fp/op-common.h (_FP_UNPACK_CANONICAL): Check > FP_DENORM_ZERO. > (_FP_CHECK_FLUSH_ZERO): New macro. > (_FP_ADD_INTERNAL): Call _FP_CHECK_FLUSH_ZERO. > (_FP_CMP): Likewise. > (_FP_CMP_EQ): Likewise. > (_FP_TO_INT): Do not set inexact for subnormal arguments if > FP_DENORM_ZERO. > (FP_EXTEND): Call _FP_CHECK_FLUSH_ZERO. > (FP_TRUNC): Likewise. Looks good to me. Small nit about the "centralized defaults" pattern being typo-prone. > diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h > index 18d6e31..3deb9b1 100644 > --- a/soft-fp/op-common.h > +++ b/soft-fp/op-common.h > @@ -63,6 +63,12 @@ > case 0: \ > if (_FP_FRAC_ZEROP_##wc (X)) \ > X##_c = FP_CLS_ZERO; \ > + else if (FP_DENORM_ZERO) \ > + { \ > + X##_c = FP_CLS_ZERO; \ > + _FP_FRAC_SET_##wc (X, _FP_ZEROFRAC_##wc); \ > + FP_SET_EXCEPTION (FP_EX_DENORM); \ OK. > + } \ > else \ > { \ > /* A denormalized number. */ \ > @@ -100,6 +106,21 @@ > other classification is not done. */ > #define _FP_UNPACK_SEMIRAW(fs, wc, X) _FP_FRAC_SLL_##wc (X, _FP_WORKBITS) > > +/* Check whether a raw or semi-raw input value should be flushed to > + zero, and flush it to zero if so. */ > +#define _FP_CHECK_FLUSH_ZERO(fs, wc, X) \ > + do \ > + { \ > + if (FP_DENORM_ZERO \ > + && X##_e == 0 \ > + && !_FP_FRAC_ZEROP_##wc (X)) \ > + { \ > + _FP_FRAC_SET_##wc (X, _FP_ZEROFRAC_##wc); \ > + FP_SET_EXCEPTION (FP_EX_DENORM); \ OK. > + } \ > + } \ > + while (0) > + > /* A semi-raw value has overflowed to infinity. Adjust the mantissa > and exponent appropriately. */ > #define _FP_OVERFLOW_SEMIRAW(fs, wc, X) \ > @@ -388,6 +409,8 @@ > #define _FP_ADD_INTERNAL(fs, wc, R, X, Y, OP) \ > do \ > { \ > + _FP_CHECK_FLUSH_ZERO (fs, wc, X); \ > + _FP_CHECK_FLUSH_ZERO (fs, wc, Y); \ OK. > if (X##_s == Y##_s) \ > { \ > /* Addition. */ \ > @@ -1235,6 +1258,9 @@ > int _FP_CMP_is_zero_x; \ > int _FP_CMP_is_zero_y; \ > \ > + _FP_CHECK_FLUSH_ZERO (fs, wc, X); \ > + _FP_CHECK_FLUSH_ZERO (fs, wc, Y); \ OK. > + \ > _FP_CMP_is_zero_x \ > = (!X##_e && _FP_FRAC_ZEROP_##wc (X)) ? 1 : 0; \ > _FP_CMP_is_zero_y \ > @@ -1277,6 +1303,9 @@ > } \ > else \ > { \ > + _FP_CHECK_FLUSH_ZERO (fs, wc, X); \ > + _FP_CHECK_FLUSH_ZERO (fs, wc, Y); \ OK. > + \ > ret = !(X##_e == Y##_e \ > && _FP_FRAC_EQ_##wc (X, Y) \ > && (X##_s == Y##_s || (!X##_e && _FP_FRAC_ZEROP_##wc (X)))); \ > @@ -1374,7 +1403,8 @@ > { \ > if (!_FP_FRAC_ZEROP_##wc (X)) \ > { \ > - FP_SET_EXCEPTION (FP_EX_INEXACT); \ > + if (!FP_DENORM_ZERO) \ > + FP_SET_EXCEPTION (FP_EX_INEXACT); \ OK. > FP_SET_EXCEPTION (FP_EX_DENORM); \ > } \ > } \ > @@ -1558,6 +1588,7 @@ > { \ > if (S##_e == 0) \ > { \ > + _FP_CHECK_FLUSH_ZERO (sfs, swc, S); \ OK. > if (_FP_FRAC_ZEROP_##swc (S)) \ > D##_e = 0; \ > else if (_FP_EXPBIAS_##dfs \ > @@ -1644,6 +1675,7 @@ > { \ > if (S##_e == 0) \ > { \ > + _FP_CHECK_FLUSH_ZERO (sfs, swc, S); \ OK. > D##_e = 0; \ > if (_FP_FRAC_ZEROP_##swc (S)) \ > _FP_FRAC_SET_##dwc (D, _FP_ZEROFRAC_##dwc); \ > diff --git a/soft-fp/soft-fp.h b/soft-fp/soft-fp.h > index 86bc7fa..05fcca0 100644 > --- a/soft-fp/soft-fp.h > +++ b/soft-fp/soft-fp.h > @@ -161,6 +161,11 @@ > # define FP_HANDLE_EXCEPTIONS do {} while (0) > #endif > > +/* Whether to flush subnormal inputs to zero with the same sign. */ > +#ifndef FP_DENORM_ZERO > +# define FP_DENORM_ZERO 0 > +#endif OK, but... For the record this is an instance of the "centralized defaults" pattern which we try to avoid in glibc. What if the target accidentally defines: #define FP_DENORM_ZER 1 or #define FP_DENRM_ZERO 1 See: https://sourceware.org/glibc/wiki/Wundef > + > #ifndef FP_INHIBIT_RESULTS > /* By default we write the results always. > sfp-machine may override this and e.g. > Cheers, Carlos.
On Wed, 8 Oct 2014, Carlos O'Donell wrote: > > diff --git a/soft-fp/soft-fp.h b/soft-fp/soft-fp.h > > index 86bc7fa..05fcca0 100644 > > --- a/soft-fp/soft-fp.h > > +++ b/soft-fp/soft-fp.h > > @@ -161,6 +161,11 @@ > > # define FP_HANDLE_EXCEPTIONS do {} while (0) > > #endif > > > > +/* Whether to flush subnormal inputs to zero with the same sign. */ > > +#ifndef FP_DENORM_ZERO > > +# define FP_DENORM_ZERO 0 > > +#endif > > OK, but... > > For the record this is an instance of the "centralized defaults" pattern > which we try to avoid in glibc. What if the target accidentally defines: I am unconvinced this can readily be avoided in the soft-fp code without a lot of duplication of common definitions in different sfp-machine.h files (of which there are a lot, when you consider those in libgcc and the Linux kernel as well as glibc).
On 10/08/2014 08:57 PM, Joseph S. Myers wrote: > On Wed, 8 Oct 2014, Carlos O'Donell wrote: > >>> diff --git a/soft-fp/soft-fp.h b/soft-fp/soft-fp.h >>> index 86bc7fa..05fcca0 100644 >>> --- a/soft-fp/soft-fp.h >>> +++ b/soft-fp/soft-fp.h >>> @@ -161,6 +161,11 @@ >>> # define FP_HANDLE_EXCEPTIONS do {} while (0) >>> #endif >>> >>> +/* Whether to flush subnormal inputs to zero with the same sign. */ >>> +#ifndef FP_DENORM_ZERO >>> +# define FP_DENORM_ZERO 0 >>> +#endif >> >> OK, but... >> >> For the record this is an instance of the "centralized defaults" pattern >> which we try to avoid in glibc. What if the target accidentally defines: > > I am unconvinced this can readily be avoided in the soft-fp code without a > lot of duplication of common definitions in different sfp-machine.h files > (of which there are a lot, when you consider those in libgcc and the Linux > kernel as well as glibc). I am not sufficiently familiar with the code to make that judgement, I was only noting that we're trying to avoid that kind of pattern in glibc in general. However, "in general" may not apply to this specific case for soft-fp. Cheers, Carlos.
diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h index 18d6e31..3deb9b1 100644 --- a/soft-fp/op-common.h +++ b/soft-fp/op-common.h @@ -63,6 +63,12 @@ case 0: \ if (_FP_FRAC_ZEROP_##wc (X)) \ X##_c = FP_CLS_ZERO; \ + else if (FP_DENORM_ZERO) \ + { \ + X##_c = FP_CLS_ZERO; \ + _FP_FRAC_SET_##wc (X, _FP_ZEROFRAC_##wc); \ + FP_SET_EXCEPTION (FP_EX_DENORM); \ + } \ else \ { \ /* A denormalized number. */ \ @@ -100,6 +106,21 @@ other classification is not done. */ #define _FP_UNPACK_SEMIRAW(fs, wc, X) _FP_FRAC_SLL_##wc (X, _FP_WORKBITS) +/* Check whether a raw or semi-raw input value should be flushed to + zero, and flush it to zero if so. */ +#define _FP_CHECK_FLUSH_ZERO(fs, wc, X) \ + do \ + { \ + if (FP_DENORM_ZERO \ + && X##_e == 0 \ + && !_FP_FRAC_ZEROP_##wc (X)) \ + { \ + _FP_FRAC_SET_##wc (X, _FP_ZEROFRAC_##wc); \ + FP_SET_EXCEPTION (FP_EX_DENORM); \ + } \ + } \ + while (0) + /* A semi-raw value has overflowed to infinity. Adjust the mantissa and exponent appropriately. */ #define _FP_OVERFLOW_SEMIRAW(fs, wc, X) \ @@ -388,6 +409,8 @@ #define _FP_ADD_INTERNAL(fs, wc, R, X, Y, OP) \ do \ { \ + _FP_CHECK_FLUSH_ZERO (fs, wc, X); \ + _FP_CHECK_FLUSH_ZERO (fs, wc, Y); \ if (X##_s == Y##_s) \ { \ /* Addition. */ \ @@ -1235,6 +1258,9 @@ int _FP_CMP_is_zero_x; \ int _FP_CMP_is_zero_y; \ \ + _FP_CHECK_FLUSH_ZERO (fs, wc, X); \ + _FP_CHECK_FLUSH_ZERO (fs, wc, Y); \ + \ _FP_CMP_is_zero_x \ = (!X##_e && _FP_FRAC_ZEROP_##wc (X)) ? 1 : 0; \ _FP_CMP_is_zero_y \ @@ -1277,6 +1303,9 @@ } \ else \ { \ + _FP_CHECK_FLUSH_ZERO (fs, wc, X); \ + _FP_CHECK_FLUSH_ZERO (fs, wc, Y); \ + \ ret = !(X##_e == Y##_e \ && _FP_FRAC_EQ_##wc (X, Y) \ && (X##_s == Y##_s || (!X##_e && _FP_FRAC_ZEROP_##wc (X)))); \ @@ -1374,7 +1403,8 @@ { \ if (!_FP_FRAC_ZEROP_##wc (X)) \ { \ - FP_SET_EXCEPTION (FP_EX_INEXACT); \ + if (!FP_DENORM_ZERO) \ + FP_SET_EXCEPTION (FP_EX_INEXACT); \ FP_SET_EXCEPTION (FP_EX_DENORM); \ } \ } \ @@ -1558,6 +1588,7 @@ { \ if (S##_e == 0) \ { \ + _FP_CHECK_FLUSH_ZERO (sfs, swc, S); \ if (_FP_FRAC_ZEROP_##swc (S)) \ D##_e = 0; \ else if (_FP_EXPBIAS_##dfs \ @@ -1644,6 +1675,7 @@ { \ if (S##_e == 0) \ { \ + _FP_CHECK_FLUSH_ZERO (sfs, swc, S); \ D##_e = 0; \ if (_FP_FRAC_ZEROP_##swc (S)) \ _FP_FRAC_SET_##dwc (D, _FP_ZEROFRAC_##dwc); \ diff --git a/soft-fp/soft-fp.h b/soft-fp/soft-fp.h index 86bc7fa..05fcca0 100644 --- a/soft-fp/soft-fp.h +++ b/soft-fp/soft-fp.h @@ -161,6 +161,11 @@ # define FP_HANDLE_EXCEPTIONS do {} while (0) #endif +/* Whether to flush subnormal inputs to zero with the same sign. */ +#ifndef FP_DENORM_ZERO +# define FP_DENORM_ZERO 0 +#endif + #ifndef FP_INHIBIT_RESULTS /* By default we write the results always. sfp-machine may override this and e.g.