| Message ID | Pine.LNX.4.64.1409190128300.20342@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
On 09/18/2014 09:29 PM, Joseph S. Myers wrote: > Here is a corrected version of this patch with a bug in _FP_TO_INT's sNaN > detection fixed. Ignoring original patch and reviewing this one. The changes in indentation make this patch annoying to review. Your changes to _FP_CMP_CHECK_NAN are IMO too complicated to follow now. OK from me only if you simplify _FP_CMP_CHECK_NAN in *some* way to make it easier to read, maintain and review. > diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h > index af859e2..833658d 100644 > --- a/soft-fp/op-common.h > +++ b/soft-fp/op-common.h > @@ -87,7 +87,8 @@ > X##_c = FP_CLS_NAN; \ > /* Check for signaling NaN. */ \ > if (_FP_FRAC_SNANP (fs, X)) \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID \ > + | FP_EX_INVALID_SNAN); \ OK. > } \ > break; \ > } \ > @@ -123,14 +124,14 @@ > > /* Check for a semi-raw value being a signaling NaN and raise the > invalid exception if so. */ > -#define _FP_CHECK_SIGNAN_SEMIRAW(fs, wc, X) \ > - do \ > - { \ > - if (X##_e == _FP_EXPMAX_##fs \ > - && !_FP_FRAC_ZEROP_##wc (X) \ > - && _FP_FRAC_SNANP_SEMIRAW (fs, X)) \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > - } \ > +#define _FP_CHECK_SIGNAN_SEMIRAW(fs, wc, X) \ > + do \ > + { \ > + if (X##_e == _FP_EXPMAX_##fs \ > + && !_FP_FRAC_ZEROP_##wc (X) \ > + && _FP_FRAC_SNANP_SEMIRAW (fs, X)) \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SNAN); \ OK. > + } \ > while (0) > > /* Choose a NaN result from an operation on two semi-raw NaN > @@ -741,7 +742,8 @@ > R##_s = _FP_NANSIGN_##fs; \ > _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ > _FP_FRAC_SLL_##wc (R, _FP_WORKBITS); \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID \ > + | FP_EX_INVALID_ISI); \ OK. > } \ > else \ > { \ > @@ -893,7 +895,7 @@ > R##_s = _FP_NANSIGN_##fs; \ > R##_c = FP_CLS_NAN; \ > _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_IMZ); \ OK. > break; \ > \ > default: \ > @@ -1057,7 +1059,7 @@ > _FP_FMA_T##_s = _FP_NANSIGN_##fs; \ > _FP_FMA_T##_c = FP_CLS_NAN; \ > _FP_FRAC_SET_##wc (_FP_FMA_T, _FP_NANFRAC_##fs); \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_IMZ_FMA); \ OK. > break; \ > \ > default: \ > @@ -1102,7 +1104,7 @@ > R##_s = _FP_NANSIGN_##fs; \ > R##_c = FP_CLS_NAN; \ > _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_ISI); \ OK. > } \ > break; \ > \ > @@ -1176,7 +1178,10 @@ > R##_s = _FP_NANSIGN_##fs; \ > R##_c = FP_CLS_NAN; \ > _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID \ > + | (X##_c == FP_CLS_INF \ > + ? FP_EX_INVALID_IDI \ > + : FP_EX_INVALID_ZDZ)); \ OK. > break; \ > \ > default: \ > @@ -1190,17 +1195,25 @@ > raise exceptions for signaling NaN operands, 2 to raise exceptions > for all NaN operands. */ > > -#define _FP_CMP_CHECK_NAN(fs, wc, X, Y, ex) \ > - do \ > - { \ > - if (ex) \ > - { \ > - if ((ex) == 2 \ > - || _FP_ISSIGNAN (fs, wc, X) \ > - || _FP_ISSIGNAN (fs, wc, Y)) \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > - } \ > - } \ > +#define _FP_CMP_CHECK_NAN(fs, wc, X, Y, ex) \ > + do \ > + { \ > + if (ex) \ > + { \ > + if (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) \ > + { \ > + if ((ex) == 2) \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_VC); \ > + if (_FP_ISSIGNAN (fs, wc, X) \ > + || _FP_ISSIGNAN (fs, wc, Y)) \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SNAN); \ > + } \ > + else if ((ex) == 2 \ > + || _FP_ISSIGNAN (fs, wc, X) \ > + || _FP_ISSIGNAN (fs, wc, Y)) \ > + FP_SET_EXCEPTION (FP_EX_INVALID); \ > + } \ This is no longer OK for me. This is really hard to follow now. This should really be broken out logically into a series of checks that we leave up to the compiler to reorganize. Unless you can show that such a source setup produces drastically worse code. > + } \ > while (0) > > /* Main differential comparison routine. The inputs should be raw not > @@ -1285,58 +1298,58 @@ > > /* Main square root routine. The input value should be cooked. */ > > -#define _FP_SQRT(fs, wc, R, X) \ > - do \ > - { \ > - _FP_FRAC_DECL_##wc (_FP_SQRT_T); \ > - _FP_FRAC_DECL_##wc (_FP_SQRT_S); \ > - _FP_W_TYPE _FP_SQRT_q; \ > - switch (X##_c) \ > - { \ > - case FP_CLS_NAN: \ > - _FP_FRAC_COPY_##wc (R, X); \ > - R##_s = X##_s; \ > - R##_c = FP_CLS_NAN; \ > - break; \ > - case FP_CLS_INF: \ > - if (X##_s) \ > - { \ > - R##_s = _FP_NANSIGN_##fs; \ > - R##_c = FP_CLS_NAN; /* NAN */ \ > - _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > - } \ > - else \ > - { \ > - R##_s = 0; \ > - R##_c = FP_CLS_INF; /* sqrt(+inf) = +inf */ \ > - } \ > - break; \ > - case FP_CLS_ZERO: \ > - R##_s = X##_s; \ > - R##_c = FP_CLS_ZERO; /* sqrt(+-0) = +-0 */ \ > - break; \ > - case FP_CLS_NORMAL: \ > - R##_s = 0; \ > - if (X##_s) \ > - { \ > - R##_c = FP_CLS_NAN; /* NAN */ \ > - R##_s = _FP_NANSIGN_##fs; \ > - _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > - break; \ > - } \ > - R##_c = FP_CLS_NORMAL; \ > - if (X##_e & 1) \ > - _FP_FRAC_SLL_##wc (X, 1); \ > - R##_e = X##_e >> 1; \ > - _FP_FRAC_SET_##wc (_FP_SQRT_S, _FP_ZEROFRAC_##wc); \ > - _FP_FRAC_SET_##wc (R, _FP_ZEROFRAC_##wc); \ > - _FP_SQRT_q = _FP_OVERFLOW_##fs >> 1; \ > - _FP_SQRT_MEAT_##wc (R, _FP_SQRT_S, _FP_SQRT_T, X, \ > - _FP_SQRT_q); \ > - } \ > - } \ The change in indentation makes this hard to review. Please try to avoid it. > +#define _FP_SQRT(fs, wc, R, X) \ > + do \ > + { \ > + _FP_FRAC_DECL_##wc (_FP_SQRT_T); \ > + _FP_FRAC_DECL_##wc (_FP_SQRT_S); \ > + _FP_W_TYPE _FP_SQRT_q; \ > + switch (X##_c) \ > + { \ > + case FP_CLS_NAN: \ > + _FP_FRAC_COPY_##wc (R, X); \ > + R##_s = X##_s; \ > + R##_c = FP_CLS_NAN; \ > + break; \ > + case FP_CLS_INF: \ > + if (X##_s) \ > + { \ > + R##_s = _FP_NANSIGN_##fs; \ > + R##_c = FP_CLS_NAN; /* NAN */ \ > + _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SQRT); \ > + } \ > + else \ > + { \ > + R##_s = 0; \ > + R##_c = FP_CLS_INF; /* sqrt(+inf) = +inf */ \ > + } \ > + break; \ > + case FP_CLS_ZERO: \ > + R##_s = X##_s; \ > + R##_c = FP_CLS_ZERO; /* sqrt(+-0) = +-0 */ \ > + break; \ > + case FP_CLS_NORMAL: \ > + R##_s = 0; \ > + if (X##_s) \ > + { \ > + R##_c = FP_CLS_NAN; /* NAN */ \ > + R##_s = _FP_NANSIGN_##fs; \ > + _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SQRT); \ OK. Change is here for FP_EX_INVALID_SQRT. > + break; \ > + } \ > + R##_c = FP_CLS_NORMAL; \ > + if (X##_e & 1) \ > + _FP_FRAC_SLL_##wc (X, 1); \ > + R##_e = X##_e >> 1; \ > + _FP_FRAC_SET_##wc (_FP_SQRT_S, _FP_ZEROFRAC_##wc); \ > + _FP_FRAC_SET_##wc (R, _FP_ZEROFRAC_##wc); \ > + _FP_SQRT_q = _FP_OVERFLOW_##fs >> 1; \ > + _FP_SQRT_MEAT_##wc (R, _FP_SQRT_S, _FP_SQRT_T, X, \ > + _FP_SQRT_q); \ > + } \ > + } \ OK. > while (0) > > /* Convert from FP to integer. Input is raw. */ > @@ -1399,12 +1412,17 @@ > }) \ > : 0); \ > if (!_FP_FRAC_ZEROP_##wc (X)) \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_CVI); \ > else if (_FP_TO_INT_inexact) \ > FP_SET_EXCEPTION (FP_EX_INEXACT); \ > } \ > else \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID \ > + | FP_EX_INVALID_CVI \ > + | ((FP_EX_INVALID_SNAN \ > + && _FP_ISSIGNAN (fs, wc, X)) \ > + ? FP_EX_INVALID_SNAN \ > + : 0)); \ OK. This has the correction to raise the FP_EX_INVALID_SNAN correctly. > } \ > else \ > { \ > @@ -1563,7 +1581,8 @@ > if (!_FP_FRAC_ZEROP_##swc (S)) \ > { \ > if (_FP_FRAC_SNANP (sfs, S)) \ > - FP_SET_EXCEPTION (FP_EX_INVALID); \ > + FP_SET_EXCEPTION (FP_EX_INVALID \ > + | FP_EX_INVALID_SNAN); \ OK. > _FP_FRAC_SLL_##dwc (D, (_FP_FRACBITS_##dfs \ > - _FP_FRACBITS_##sfs)); \ > _FP_SETQNAN (dfs, dwc, D); \ > diff --git a/soft-fp/soft-fp.h b/soft-fp/soft-fp.h > index 5fb7358..3ced6ca 100644 > --- a/soft-fp/soft-fp.h > +++ b/soft-fp/soft-fp.h > @@ -83,6 +83,44 @@ > # define FP_EX_DENORM 0 > #endif > > +/* Sub-exceptions of "invalid". */ > +/* Signaling NaN operand. */ > +#ifndef FP_EX_INVALID_SNAN > +# define FP_EX_INVALID_SNAN 0 > +#endif > +/* Inf * 0. */ > +#ifndef FP_EX_INVALID_IMZ > +# define FP_EX_INVALID_IMZ 0 > +#endif > +/* fma (Inf, 0, c). */ > +#ifndef FP_EX_INVALID_IMZ_FMA > +# define FP_EX_INVALID_IMZ_FMA 0 > +#endif > +/* Inf - Inf. */ > +#ifndef FP_EX_INVALID_ISI > +# define FP_EX_INVALID_ISI 0 > +#endif > +/* 0 / 0. */ > +#ifndef FP_EX_INVALID_ZDZ > +# define FP_EX_INVALID_ZDZ 0 > +#endif > +/* Inf / Inf. */ > +#ifndef FP_EX_INVALID_IDI > +# define FP_EX_INVALID_IDI 0 > +#endif > +/* sqrt (negative). */ > +#ifndef FP_EX_INVALID_SQRT > +# define FP_EX_INVALID_SQRT 0 > +#endif > +/* Invalid conversion to integer. */ > +#ifndef FP_EX_INVALID_CVI > +# define FP_EX_INVALID_CVI 0 > +#endif > +/* Invalid comparison. */ > +#ifndef FP_EX_INVALID_VC > +# define FP_EX_INVALID_VC 0 > +#endif > + OK. > /* _FP_STRUCT_LAYOUT may be defined as an attribute to determine the > struct layout variant used for structures where bit-fields are used > to access specific parts of binary floating-point numbers. This is > Cheers, Carlos.
On Wed, 8 Oct 2014, Carlos O'Donell wrote: > On 09/18/2014 09:29 PM, Joseph S. Myers wrote: > > Here is a corrected version of this patch with a bug in _FP_TO_INT's sNaN > > detection fixed. > > Ignoring original patch and reviewing this one. > > The changes in indentation make this patch annoying to review. > > Your changes to _FP_CMP_CHECK_NAN are IMO too complicated > to follow now. _FP_CMP_CHECK_NAN is much simpler than most of the code in soft-fp, where typically you need to check for off-by-one errors in exponent manipulations that are written so that as much as possible gets folded to a single constant (without the compiler needing first to deduce that undefined integer overflow can't occur in a particular case). > > +#define _FP_CMP_CHECK_NAN(fs, wc, X, Y, ex) \ > > + do \ > > + { \ > > + if (ex) \ > > + { \ > > + if (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) \ > > + { \ > > + if ((ex) == 2) \ > > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_VC); \ > > + if (_FP_ISSIGNAN (fs, wc, X) \ > > + || _FP_ISSIGNAN (fs, wc, Y)) \ > > + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SNAN); \ > > + } \ > > + else if ((ex) == 2 \ > > + || _FP_ISSIGNAN (fs, wc, X) \ > > + || _FP_ISSIGNAN (fs, wc, Y)) \ > > + FP_SET_EXCEPTION (FP_EX_INVALID); \ > > + } \ > > This is no longer OK for me. This is really hard to follow now. > > This should really be broken out logically into a series of checks > that we leave up to the compiler to reorganize. Unless you can show > that such a source setup produces drastically worse code. The broken-out case is the if (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) one - in that case, the two possible causes of exceptions are checked for separately. That case would in fact be valid unconditionally (inside the "if (ex)", but with the (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) condition and the "else" case removed). However, to avoid code generation changes to make the patch more obviously safe, and to make it obvious the checks for signaling NaNs will get optimized out when (ex) == 2 and the separate cases of "invalid" aren't being distinguished, the general broken-out case is separated from the common case where different cases of "invalid" exceptions aren't distinguished. Perhaps more comments would help here? Along the lines of do { /* The arguments are unordered, which may or may not result in an exception. */ if (ex) { /* At least some cases of unordered arguments result in exceptions; check whether this is one. */ if (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) { /* Check separately for each case of "invalid" exceptions. */ } /* Otherwise, we only need to check whether to raise an exception, not which case or cases it is. */ else if (...) > The change in indentation makes this hard to review. It's not a change in indentation. It's a move of the backslashes at end of line because some lines got longer and the backslashes are all meant to be aligned with each other. In such cases it can be helpful to apply the patch then look at the changes with git diff -w.
On 10/08/2014 08:48 PM, Joseph S. Myers wrote: > On Wed, 8 Oct 2014, Carlos O'Donell wrote: > >> On 09/18/2014 09:29 PM, Joseph S. Myers wrote: >>> Here is a corrected version of this patch with a bug in _FP_TO_INT's sNaN >>> detection fixed. >> >> Ignoring original patch and reviewing this one. >> >> The changes in indentation make this patch annoying to review. >> >> Your changes to _FP_CMP_CHECK_NAN are IMO too complicated >> to follow now. > > _FP_CMP_CHECK_NAN is much simpler than most of the code in soft-fp, where > typically you need to check for off-by-one errors in exponent > manipulations that are written so that as much as possible gets folded to > a single constant (without the compiler needing first to deduce that > undefined integer overflow can't occur in a particular case). I agree that other code in soft-fp is *much* more complicated, but this code is new and you have asked for review. We are not reviewing the other code. As I mentioned before there is a lack of reviewers precisely because the code is difficult to review and understand even if you have read the IEEE standard and understand the underlying binary formats. >>> +#define _FP_CMP_CHECK_NAN(fs, wc, X, Y, ex) \ >>> + do \ >>> + { \ >>> + if (ex) \ >>> + { \ >>> + if (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) \ >>> + { \ >>> + if ((ex) == 2) \ >>> + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_VC); \ >>> + if (_FP_ISSIGNAN (fs, wc, X) \ >>> + || _FP_ISSIGNAN (fs, wc, Y)) \ >>> + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SNAN); \ >>> + } \ >>> + else if ((ex) == 2 \ >>> + || _FP_ISSIGNAN (fs, wc, X) \ >>> + || _FP_ISSIGNAN (fs, wc, Y)) \ >>> + FP_SET_EXCEPTION (FP_EX_INVALID); \ >>> + } \ >> >> This is no longer OK for me. This is really hard to follow now. >> >> This should really be broken out logically into a series of checks >> that we leave up to the compiler to reorganize. Unless you can show >> that such a source setup produces drastically worse code. > > The broken-out case is the if (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) > one - in that case, the two possible causes of exceptions are checked for > separately. > > That case would in fact be valid unconditionally (inside the "if (ex)", > but with the (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) condition and the > "else" case removed). However, to avoid code generation changes to make > the patch more obviously safe, and to make it obvious the checks for > signaling NaNs will get optimized out when (ex) == 2 and the separate > cases of "invalid" aren't being distinguished, the general broken-out case > is separated from the common case where different cases of "invalid" > exceptions aren't distinguished. > > Perhaps more comments would help here? Along the lines of > > do { > /* The arguments are unordered, which may or may not result in an > exception. */ > if (ex) > { > /* At least some cases of unordered arguments result in exceptions; check > whether this is one. */ > if (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) > { > /* Check separately for each case of "invalid" exceptions. */ > } > /* Otherwise, we only need to check whether to raise an exception, not > which case or cases it is. */ > else if (...) Almost. Add a "/* Conditionals are organized to allow the compiler to optimize away code based on the value of `ex`. */" and I'm happy. >> The change in indentation makes this hard to review. > > It's not a change in indentation. It's a move of the backslashes at end > of line because some lines got longer and the backslashes are all meant to > be aligned with each other. In such cases it can be helpful to apply the > patch then look at the changes with git diff -w. Sorry, yes, I said "indentation" but really meant: the lines changed in ways unrelated to the patch and that makes review harder. I have never used `git diff -w` and I will try it out. Cheers, Carlos.
On Wed, 8 Oct 2014, Carlos O'Donell wrote: > I agree that other code in soft-fp is *much* more complicated, but > this code is new and you have asked for review. We are not reviewing > the other code. I am (well, have done - I've reviewed all the existing soft-fp code multiple times). > As I mentioned before there is a lack of reviewers precisely because > the code is difficult to review and understand even if you have read > the IEEE standard and understand the underlying binary formats. I think the actual lack is more to do with limited interest in this code, given that on the most widely used platforms it's not used for standard floating-point types. (In the GCC context, it seems to be the Fortran community that's most interested in __float128.)
diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h index af859e2..833658d 100644 --- a/soft-fp/op-common.h +++ b/soft-fp/op-common.h @@ -87,7 +87,8 @@ X##_c = FP_CLS_NAN; \ /* Check for signaling NaN. */ \ if (_FP_FRAC_SNANP (fs, X)) \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID \ + | FP_EX_INVALID_SNAN); \ } \ break; \ } \ @@ -123,14 +124,14 @@ /* Check for a semi-raw value being a signaling NaN and raise the invalid exception if so. */ -#define _FP_CHECK_SIGNAN_SEMIRAW(fs, wc, X) \ - do \ - { \ - if (X##_e == _FP_EXPMAX_##fs \ - && !_FP_FRAC_ZEROP_##wc (X) \ - && _FP_FRAC_SNANP_SEMIRAW (fs, X)) \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ - } \ +#define _FP_CHECK_SIGNAN_SEMIRAW(fs, wc, X) \ + do \ + { \ + if (X##_e == _FP_EXPMAX_##fs \ + && !_FP_FRAC_ZEROP_##wc (X) \ + && _FP_FRAC_SNANP_SEMIRAW (fs, X)) \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SNAN); \ + } \ while (0) /* Choose a NaN result from an operation on two semi-raw NaN @@ -741,7 +742,8 @@ R##_s = _FP_NANSIGN_##fs; \ _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ _FP_FRAC_SLL_##wc (R, _FP_WORKBITS); \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID \ + | FP_EX_INVALID_ISI); \ } \ else \ { \ @@ -893,7 +895,7 @@ R##_s = _FP_NANSIGN_##fs; \ R##_c = FP_CLS_NAN; \ _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_IMZ); \ break; \ \ default: \ @@ -1057,7 +1059,7 @@ _FP_FMA_T##_s = _FP_NANSIGN_##fs; \ _FP_FMA_T##_c = FP_CLS_NAN; \ _FP_FRAC_SET_##wc (_FP_FMA_T, _FP_NANFRAC_##fs); \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_IMZ_FMA); \ break; \ \ default: \ @@ -1102,7 +1104,7 @@ R##_s = _FP_NANSIGN_##fs; \ R##_c = FP_CLS_NAN; \ _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_ISI); \ } \ break; \ \ @@ -1176,7 +1178,10 @@ R##_s = _FP_NANSIGN_##fs; \ R##_c = FP_CLS_NAN; \ _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID \ + | (X##_c == FP_CLS_INF \ + ? FP_EX_INVALID_IDI \ + : FP_EX_INVALID_ZDZ)); \ break; \ \ default: \ @@ -1190,17 +1195,25 @@ raise exceptions for signaling NaN operands, 2 to raise exceptions for all NaN operands. */ -#define _FP_CMP_CHECK_NAN(fs, wc, X, Y, ex) \ - do \ - { \ - if (ex) \ - { \ - if ((ex) == 2 \ - || _FP_ISSIGNAN (fs, wc, X) \ - || _FP_ISSIGNAN (fs, wc, Y)) \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ - } \ - } \ +#define _FP_CMP_CHECK_NAN(fs, wc, X, Y, ex) \ + do \ + { \ + if (ex) \ + { \ + if (FP_EX_INVALID_SNAN || FP_EX_INVALID_VC) \ + { \ + if ((ex) == 2) \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_VC); \ + if (_FP_ISSIGNAN (fs, wc, X) \ + || _FP_ISSIGNAN (fs, wc, Y)) \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SNAN); \ + } \ + else if ((ex) == 2 \ + || _FP_ISSIGNAN (fs, wc, X) \ + || _FP_ISSIGNAN (fs, wc, Y)) \ + FP_SET_EXCEPTION (FP_EX_INVALID); \ + } \ + } \ while (0) /* Main differential comparison routine. The inputs should be raw not @@ -1285,58 +1298,58 @@ /* Main square root routine. The input value should be cooked. */ -#define _FP_SQRT(fs, wc, R, X) \ - do \ - { \ - _FP_FRAC_DECL_##wc (_FP_SQRT_T); \ - _FP_FRAC_DECL_##wc (_FP_SQRT_S); \ - _FP_W_TYPE _FP_SQRT_q; \ - switch (X##_c) \ - { \ - case FP_CLS_NAN: \ - _FP_FRAC_COPY_##wc (R, X); \ - R##_s = X##_s; \ - R##_c = FP_CLS_NAN; \ - break; \ - case FP_CLS_INF: \ - if (X##_s) \ - { \ - R##_s = _FP_NANSIGN_##fs; \ - R##_c = FP_CLS_NAN; /* NAN */ \ - _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ - } \ - else \ - { \ - R##_s = 0; \ - R##_c = FP_CLS_INF; /* sqrt(+inf) = +inf */ \ - } \ - break; \ - case FP_CLS_ZERO: \ - R##_s = X##_s; \ - R##_c = FP_CLS_ZERO; /* sqrt(+-0) = +-0 */ \ - break; \ - case FP_CLS_NORMAL: \ - R##_s = 0; \ - if (X##_s) \ - { \ - R##_c = FP_CLS_NAN; /* NAN */ \ - R##_s = _FP_NANSIGN_##fs; \ - _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ - break; \ - } \ - R##_c = FP_CLS_NORMAL; \ - if (X##_e & 1) \ - _FP_FRAC_SLL_##wc (X, 1); \ - R##_e = X##_e >> 1; \ - _FP_FRAC_SET_##wc (_FP_SQRT_S, _FP_ZEROFRAC_##wc); \ - _FP_FRAC_SET_##wc (R, _FP_ZEROFRAC_##wc); \ - _FP_SQRT_q = _FP_OVERFLOW_##fs >> 1; \ - _FP_SQRT_MEAT_##wc (R, _FP_SQRT_S, _FP_SQRT_T, X, \ - _FP_SQRT_q); \ - } \ - } \ +#define _FP_SQRT(fs, wc, R, X) \ + do \ + { \ + _FP_FRAC_DECL_##wc (_FP_SQRT_T); \ + _FP_FRAC_DECL_##wc (_FP_SQRT_S); \ + _FP_W_TYPE _FP_SQRT_q; \ + switch (X##_c) \ + { \ + case FP_CLS_NAN: \ + _FP_FRAC_COPY_##wc (R, X); \ + R##_s = X##_s; \ + R##_c = FP_CLS_NAN; \ + break; \ + case FP_CLS_INF: \ + if (X##_s) \ + { \ + R##_s = _FP_NANSIGN_##fs; \ + R##_c = FP_CLS_NAN; /* NAN */ \ + _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SQRT); \ + } \ + else \ + { \ + R##_s = 0; \ + R##_c = FP_CLS_INF; /* sqrt(+inf) = +inf */ \ + } \ + break; \ + case FP_CLS_ZERO: \ + R##_s = X##_s; \ + R##_c = FP_CLS_ZERO; /* sqrt(+-0) = +-0 */ \ + break; \ + case FP_CLS_NORMAL: \ + R##_s = 0; \ + if (X##_s) \ + { \ + R##_c = FP_CLS_NAN; /* NAN */ \ + R##_s = _FP_NANSIGN_##fs; \ + _FP_FRAC_SET_##wc (R, _FP_NANFRAC_##fs); \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_SQRT); \ + break; \ + } \ + R##_c = FP_CLS_NORMAL; \ + if (X##_e & 1) \ + _FP_FRAC_SLL_##wc (X, 1); \ + R##_e = X##_e >> 1; \ + _FP_FRAC_SET_##wc (_FP_SQRT_S, _FP_ZEROFRAC_##wc); \ + _FP_FRAC_SET_##wc (R, _FP_ZEROFRAC_##wc); \ + _FP_SQRT_q = _FP_OVERFLOW_##fs >> 1; \ + _FP_SQRT_MEAT_##wc (R, _FP_SQRT_S, _FP_SQRT_T, X, \ + _FP_SQRT_q); \ + } \ + } \ while (0) /* Convert from FP to integer. Input is raw. */ @@ -1399,12 +1412,17 @@ }) \ : 0); \ if (!_FP_FRAC_ZEROP_##wc (X)) \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_CVI); \ else if (_FP_TO_INT_inexact) \ FP_SET_EXCEPTION (FP_EX_INEXACT); \ } \ else \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID \ + | FP_EX_INVALID_CVI \ + | ((FP_EX_INVALID_SNAN \ + && _FP_ISSIGNAN (fs, wc, X)) \ + ? FP_EX_INVALID_SNAN \ + : 0)); \ } \ else \ { \ @@ -1563,7 +1581,8 @@ if (!_FP_FRAC_ZEROP_##swc (S)) \ { \ if (_FP_FRAC_SNANP (sfs, S)) \ - FP_SET_EXCEPTION (FP_EX_INVALID); \ + FP_SET_EXCEPTION (FP_EX_INVALID \ + | FP_EX_INVALID_SNAN); \ _FP_FRAC_SLL_##dwc (D, (_FP_FRACBITS_##dfs \ - _FP_FRACBITS_##sfs)); \ _FP_SETQNAN (dfs, dwc, D); \ diff --git a/soft-fp/soft-fp.h b/soft-fp/soft-fp.h index 5fb7358..3ced6ca 100644 --- a/soft-fp/soft-fp.h +++ b/soft-fp/soft-fp.h @@ -83,6 +83,44 @@ # define FP_EX_DENORM 0 #endif +/* Sub-exceptions of "invalid". */ +/* Signaling NaN operand. */ +#ifndef FP_EX_INVALID_SNAN +# define FP_EX_INVALID_SNAN 0 +#endif +/* Inf * 0. */ +#ifndef FP_EX_INVALID_IMZ +# define FP_EX_INVALID_IMZ 0 +#endif +/* fma (Inf, 0, c). */ +#ifndef FP_EX_INVALID_IMZ_FMA +# define FP_EX_INVALID_IMZ_FMA 0 +#endif +/* Inf - Inf. */ +#ifndef FP_EX_INVALID_ISI +# define FP_EX_INVALID_ISI 0 +#endif +/* 0 / 0. */ +#ifndef FP_EX_INVALID_ZDZ +# define FP_EX_INVALID_ZDZ 0 +#endif +/* Inf / Inf. */ +#ifndef FP_EX_INVALID_IDI +# define FP_EX_INVALID_IDI 0 +#endif +/* sqrt (negative). */ +#ifndef FP_EX_INVALID_SQRT +# define FP_EX_INVALID_SQRT 0 +#endif +/* Invalid conversion to integer. */ +#ifndef FP_EX_INVALID_CVI +# define FP_EX_INVALID_CVI 0 +#endif +/* Invalid comparison. */ +#ifndef FP_EX_INVALID_VC +# define FP_EX_INVALID_VC 0 +#endif + /* _FP_STRUCT_LAYOUT may be defined as an attribute to determine the struct layout variant used for structures where bit-fields are used to access specific parts of binary floating-point numbers. This is