| Message ID | 20241204163949.1408676-13-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add remaining CORE-MATH binary32 implementations to libm | expand |
On Wed, 4 Dec 2024, Adhemerval Zanella wrote: > For some correctly rounded inputs where infinity might generate > a number (like atanf), comparing to a pre-defined constant does not > yield the expected result in all roundind modes. > > The most straightforward way to handle it would be to get the expected > result from mpfr, where it handles all the rounding mode. OK. It appears all the uses of lit_pi* in tests are in expected results, except for uses as inputs in libm-test-ceil.inc and libm-test-floor.inc. The relevant tests with infinities are for atan, atan2, cacos, cacosh, carg, casin, casinh, catan, catanh, clog, clog10. *However*, not all of those could be converted to auto-libm-test-in as-is, because some of those tests involve an exact infinity for one part of the complex result and something involving pi for the other part of the complex result (and gen-auto-libm-tests still doesn't handle exact infinite results). So additional features on top of this patch series would be needed before all such tests could be converted and most of the lit_pi* definitions removed.
On 04/12/24 14:22, Joseph Myers wrote: > On Wed, 4 Dec 2024, Adhemerval Zanella wrote: > >> For some correctly rounded inputs where infinity might generate >> a number (like atanf), comparing to a pre-defined constant does not >> yield the expected result in all roundind modes. >> >> The most straightforward way to handle it would be to get the expected >> result from mpfr, where it handles all the rounding mode. > > OK. > > It appears all the uses of lit_pi* in tests are in expected results, > except for uses as inputs in libm-test-ceil.inc and libm-test-floor.inc. > The relevant tests with infinities are for atan, atan2, cacos, cacosh, > carg, casin, casinh, catan, catanh, clog, clog10. *However*, not all of > those could be converted to auto-libm-test-in as-is, because some of those > tests involve an exact infinity for one part of the complex result and > something involving pi for the other part of the complex result (and > gen-auto-libm-tests still doesn't handle exact infinite results). So > additional features on top of this patch series would be needed before all > such tests could be converted and most of the lit_pi* definitions removed. > Indeed, I have started it to check if clog/clog10 but I stumbled on this very issue.
* Adhemerval Zanella: > For some correctly rounded inputs where infinity might generate > a number (like atanf), comparing to a pre-defined constant does not > yield the expected result in all roundind modes. Typo: roundin[g]
Dear Adhemerval, > From: Adhemerval Zanella <adhemerval.zanella@linaro.org> > Cc: DJ Delorie <dj@redhat.com>, > Joseph Myers <josmyers@redhat.com> > Date: Wed, 4 Dec 2024 13:37:47 -0300 > > For some correctly rounded inputs where infinity might generate > a number (like atanf), comparing to a pre-defined constant does not > yield the expected result in all roundind modes. roundind -> rounding (already reported I believe) > The most straightforward way to handle it would be to get the expected > result from mpfr, where it handles all the rounding mode. all the rounding mode*s* > --- > math/gen-auto-libm-tests.c | 37 ++++++++++++++++++++++++++++++------- > 1 file changed, 30 insertions(+), 7 deletions(-) > > diff --git a/math/gen-auto-libm-tests.c b/math/gen-auto-libm-tests.c > index 0b1e307fae..a60e0f6ec2 100644 > --- a/math/gen-auto-libm-tests.c > +++ b/math/gen-auto-libm-tests.c > @@ -40,12 +40,14 @@ > empty lines. > > Other lines are test lines, of the form "function input1 input2 > - ... [flag1 flag2 ...]". Inputs are either finite real numbers or > - integers, depending on the function under test. Real numbers may > - be in any form acceptable to mpfr_strtofr (base 0); integers in any > - form acceptable to mpz_set_str (base 0). In addition, real numbers > - may be certain special strings such as "pi", as listed in the > - special_real_inputs array. > + ... [flag1 flag2 ...]". Inputs are either finite real numbers, > + positive or negative infinite (in the form of "inf" or "-inf"), or is Inf or -Inf also allowed? > + integers, depending on the function under test. Real numbers may be > + in any form acceptable to mpfr_strtofr (base 0), infinite may in any may *be* > + form acceptable to mpfr_set_inf, and integers in any form acceptable I don't understand what "any form acceptable to mpfr_set_inf" means, since mpfr_set_inf takes just a sign > + to mpz_set_str (base 0). In addition, real numbers may be certain > + special strings such as "pi", as listed in the special_real_inputs is Pi also allowed? > + array. > > Each flag is a flag name possibly followed by a series of > ":condition". Conditions may be any of the names of floating-point > @@ -981,6 +983,26 @@ special_fill_e_minus_1 (mpfr_t res0, mpfr_t res1, fp_format format) > return 2; > } > > +/* Set the precision of RES0 based on FORMAT and initialize as an RES0 -> res0 and FORMAT -> format ? > + infinite number. */ > +static size_t > +special_fill_inf (mpfr_t res0, mpfr_t res1 __attribute__ ((unused)), > + fp_format format) > +{ > + mpfr_init2 (res0, fp_formats[format].mant_dig); > + mpfr_set_inf (res0, 0); > + return 1; > +} a comment would be welcome for special_fill_minus_inf > +static size_t > +special_fill_minus_inf (mpfr_t res0, mpfr_t res1 __attribute__ ((unused)), > + fp_format format) > +{ > + mpfr_init2 (res0, fp_formats[format].mant_dig); > + mpfr_set_inf (res0, -1); > + return 1; > +} > + > /* A special string accepted in input arguments. */ > typedef struct > { > @@ -1016,6 +1038,8 @@ static const special_real_input special_real_inputs[] = > { "e", special_fill_e }, > { "1/e", special_fill_1_e }, > { "e-1", special_fill_e_minus_1 }, > + { "inf", special_fill_inf }, > + { "-inf", special_fill_minus_inf }, > }; > > /* Given a real number R computed in round-to-zero mode, set the > @@ -1062,7 +1086,6 @@ round_real (mpfr_t res[rm_num_modes], > unsigned int exc_after[rm_num_modes], > mpfr_t r, fp_format format) > { > - assert (mpfr_number_p (r)); > for (rounding_mode m = rm_first_mode; m < rm_num_modes; m++) > { > mpfr_init2 (res[m], fp_formats[format].mant_dig); ok modulo the comments above. Paul
On 05/12/24 11:43, Paul Zimmermann wrote: > Dear Adhemerval, > >> From: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> Cc: DJ Delorie <dj@redhat.com>, >> Joseph Myers <josmyers@redhat.com> >> Date: Wed, 4 Dec 2024 13:37:47 -0300 >> >> For some correctly rounded inputs where infinity might generate >> a number (like atanf), comparing to a pre-defined constant does not >> yield the expected result in all roundind modes. > > roundind -> rounding (already reported I believe) Ack. > >> The most straightforward way to handle it would be to get the expected >> result from mpfr, where it handles all the rounding mode. > > all the rounding mode*s* Ack. > >> --- >> math/gen-auto-libm-tests.c | 37 ++++++++++++++++++++++++++++++------- >> 1 file changed, 30 insertions(+), 7 deletions(-) >> >> diff --git a/math/gen-auto-libm-tests.c b/math/gen-auto-libm-tests.c >> index 0b1e307fae..a60e0f6ec2 100644 >> --- a/math/gen-auto-libm-tests.c >> +++ b/math/gen-auto-libm-tests.c >> @@ -40,12 +40,14 @@ >> empty lines. >> >> Other lines are test lines, of the form "function input1 input2 >> - ... [flag1 flag2 ...]". Inputs are either finite real numbers or >> - integers, depending on the function under test. Real numbers may >> - be in any form acceptable to mpfr_strtofr (base 0); integers in any >> - form acceptable to mpz_set_str (base 0). In addition, real numbers >> - may be certain special strings such as "pi", as listed in the >> - special_real_inputs array. >> + ... [flag1 flag2 ...]". Inputs are either finite real numbers, >> + positive or negative infinite (in the form of "inf" or "-inf"), or > > is Inf or -Inf also allowed? No, only the exact 'inf'/'-inf' strings. This is used internally for testing generation, so I think there is no much gain in allowing multiple way to define the same thing (although it should be as simple as add another line to special_real_inputs). > >> + integers, depending on the function under test. Real numbers may be >> + in any form acceptable to mpfr_strtofr (base 0), infinite may in any > > may *be* Ack. > >> + form acceptable to mpfr_set_inf, and integers in any form acceptable > > I don't understand what "any form acceptable to mpfr_set_inf" means, since > mpfr_set_inf takes just a sign It does not make sense indeed, I will remove this sentence. > >> + to mpz_set_str (base 0). In addition, real numbers may be certain >> + special strings such as "pi", as listed in the special_real_inputs > > is Pi also allowed? No, only the strings listed on special_real_inputs. > >> + array. >> >> Each flag is a flag name possibly followed by a series of >> ":condition". Conditions may be any of the names of floating-point >> @@ -981,6 +983,26 @@ special_fill_e_minus_1 (mpfr_t res0, mpfr_t res1, fp_format format) >> return 2; >> } >> >> +/* Set the precision of RES0 based on FORMAT and initialize as an > > RES0 -> res0 and FORMAT -> format ? This is an internal convention to uppercase the argument names on function description. > >> + infinite number. */ >> +static size_t >> +special_fill_inf (mpfr_t res0, mpfr_t res1 __attribute__ ((unused)), >> + fp_format format) >> +{ >> + mpfr_init2 (res0, fp_formats[format].mant_dig); >> + mpfr_set_inf (res0, 0); >> + return 1; >> +} > > a comment would be welcome for special_fill_minus_inf I added: /* Same as special_fill_inf, but set the sign of infinite as negative. */ > >> +static size_t >> +special_fill_minus_inf (mpfr_t res0, mpfr_t res1 __attribute__ ((unused)), >> + fp_format format) >> +{ >> + mpfr_init2 (res0, fp_formats[format].mant_dig); >> + mpfr_set_inf (res0, -1); >> + return 1; >> +} >> + >> /* A special string accepted in input arguments. */ >> typedef struct >> { >> @@ -1016,6 +1038,8 @@ static const special_real_input special_real_inputs[] = >> { "e", special_fill_e }, >> { "1/e", special_fill_1_e }, >> { "e-1", special_fill_e_minus_1 }, >> + { "inf", special_fill_inf }, >> + { "-inf", special_fill_minus_inf }, >> }; >> >> /* Given a real number R computed in round-to-zero mode, set the >> @@ -1062,7 +1086,6 @@ round_real (mpfr_t res[rm_num_modes], >> unsigned int exc_after[rm_num_modes], >> mpfr_t r, fp_format format) >> { >> - assert (mpfr_number_p (r)); >> for (rounding_mode m = rm_first_mode; m < rm_num_modes; m++) >> { >> mpfr_init2 (res[m], fp_formats[format].mant_dig); > > ok modulo the comments above. Thanks. > > Paul >
Dear Adhemerval, I agree with your changes, thanks! Paul
diff --git a/math/gen-auto-libm-tests.c b/math/gen-auto-libm-tests.c index 0b1e307fae..a60e0f6ec2 100644 --- a/math/gen-auto-libm-tests.c +++ b/math/gen-auto-libm-tests.c @@ -40,12 +40,14 @@ empty lines. Other lines are test lines, of the form "function input1 input2 - ... [flag1 flag2 ...]". Inputs are either finite real numbers or - integers, depending on the function under test. Real numbers may - be in any form acceptable to mpfr_strtofr (base 0); integers in any - form acceptable to mpz_set_str (base 0). In addition, real numbers - may be certain special strings such as "pi", as listed in the - special_real_inputs array. + ... [flag1 flag2 ...]". Inputs are either finite real numbers, + positive or negative infinite (in the form of "inf" or "-inf"), or + integers, depending on the function under test. Real numbers may be + in any form acceptable to mpfr_strtofr (base 0), infinite may in any + form acceptable to mpfr_set_inf, and integers in any form acceptable + to mpz_set_str (base 0). In addition, real numbers may be certain + special strings such as "pi", as listed in the special_real_inputs + array. Each flag is a flag name possibly followed by a series of ":condition". Conditions may be any of the names of floating-point @@ -981,6 +983,26 @@ special_fill_e_minus_1 (mpfr_t res0, mpfr_t res1, fp_format format) return 2; } +/* Set the precision of RES0 based on FORMAT and initialize as an + infinite number. */ +static size_t +special_fill_inf (mpfr_t res0, mpfr_t res1 __attribute__ ((unused)), + fp_format format) +{ + mpfr_init2 (res0, fp_formats[format].mant_dig); + mpfr_set_inf (res0, 0); + return 1; +} + +static size_t +special_fill_minus_inf (mpfr_t res0, mpfr_t res1 __attribute__ ((unused)), + fp_format format) +{ + mpfr_init2 (res0, fp_formats[format].mant_dig); + mpfr_set_inf (res0, -1); + return 1; +} + /* A special string accepted in input arguments. */ typedef struct { @@ -1016,6 +1038,8 @@ static const special_real_input special_real_inputs[] = { "e", special_fill_e }, { "1/e", special_fill_1_e }, { "e-1", special_fill_e_minus_1 }, + { "inf", special_fill_inf }, + { "-inf", special_fill_minus_inf }, }; /* Given a real number R computed in round-to-zero mode, set the @@ -1062,7 +1086,6 @@ round_real (mpfr_t res[rm_num_modes], unsigned int exc_after[rm_num_modes], mpfr_t r, fp_format format) { - assert (mpfr_number_p (r)); for (rounding_mode m = rm_first_mode; m < rm_num_modes; m++) { mpfr_init2 (res[m], fp_formats[format].mant_dig);