Message ID | 1481563942-19034-1-git-send-email-gftg@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On Mon, 12 Dec 2016, Gabriel F. T. Gomes wrote: > In order to support float128 tests, the calls to snprintf, which does > not support the type __float128, are replaced with calls to > strfrom{f,d,l}. This patch appears to lose the effects of the ' ' flag, i.e. aligning positive and negative values in the output. I think that alignment is desirable, meaning you need another level of wrappers to handle moving the output of FTOSTR and prepending it with a space in the case where a '-' was not output. This is relevant both to the cases where you're inserting a precision in a format string, and to those where the precision was already embedded in the string. Also, the PRINTF_EXPR, PRINTF_XEXPR, PRINTF_NEXPR macros only made sense at all in a context where snprintf was used. With the use of strfrom, they don't depend on the type used, and the code would be clearer if it just used "e", "a", "f" directly in libm-test.inc (with the macro definitions and the comments about them in libm-test.inc being removed). (Actually I think there are just four cases involved, which would indicate further refactoring: "% .*e" with TYPE_DECIMAL_DIG - 1 precision, "% .*a" with TYPE_HEX_DIG - 1 precision, "%.0f" and "% .4f". The third of these should work unchanged with strfrom. The fourth is only used with nonnegative values, so a space could be inserted in the users instead of needing special handling to emulate the ' ' flag. But I don't think such further refactoring is needed for this change to go in, as long as you keep output unchanged and eliminate type-specific definitions of macros PRINTF_* when all types have the same definitions.) > +/* The definitions TYPE_DECIMAL_DIG and TYPE_HEX_DIG are used to select the > + precision (i.e.: number of fractional digits) to be printed. When using > + snprintf, it is possible to pass the precision in an argument with "%.*". > + On the other hand, strfrom does not accept such format string, thus the > + precision must be coded in the format string itself. */ > +static int > +fmt_ftostr (char *dest, size_t size, const char *format, > + const char *conversion, int precision, FLOAT value) The function comment should describe what the function does, including the semantics of its arguments and return value. > + /* Generate the format string. */ > + ptr_format = stpcpy(new_format, format); > + ret = sprintf(ptr_format, "%d", precision); > + ptr_format += ret; > + ptr_format = stpcpy(ptr_format, conversion); > + > + /* Call the float to string conversion function, e.g.: strfromd. */ > + return FTOSTR(dest, size, new_format, value); Missing spaces before '(', several times. > - FTOSTR (fstrn, FSTR_MAX, "% .*" PRINTF_EXPR, TYPE_DECIMAL_DIG - 1, f); > - FTOSTR (fstrx, FSTR_MAX, "% .*" PRINTF_XEXPR, TYPE_HEX_DIG - 1, f); > + fmt_ftostr (fstrn, FSTR_MAX, "%.", PRINTF_EXPR, TYPE_DECIMAL_DIG - 1, f); > + fmt_ftostr (fstrx, FSTR_MAX, "%.", PRINTF_XEXPR, TYPE_HEX_DIG - 1, f); These are examples of cases where I think the space-padding for non-negative results should be preserved (possibly by making fmt_ftostr do that padding, if it's only ever used in cases where ' ' was used before). You could e.g. compare test-ldouble.out (as an example of nontrivial output on powerpc) before and after the changes to make sure there aren't unexpected changes to the output of the tests.
diff --git a/math/libm-test.inc b/math/libm-test.inc index e973a3f..500bbed 100644 --- a/math/libm-test.inc +++ b/math/libm-test.inc @@ -355,6 +355,29 @@ static FLOAT max_valid_error; #define TYPE_DECIMAL_DIG __CONCATX (PREFIX, _DECIMAL_DIG) #define TYPE_HEX_DIG ((MANT_DIG + 6) / 4) +/* The definitions TYPE_DECIMAL_DIG and TYPE_HEX_DIG are used to select the + precision (i.e.: number of fractional digits) to be printed. When using + snprintf, it is possible to pass the precision in an argument with "%.*". + On the other hand, strfrom does not accept such format string, thus the + precision must be coded in the format string itself. */ +static int +fmt_ftostr (char *dest, size_t size, const char *format, + const char *conversion, int precision, FLOAT value) +{ + char new_format[64]; + char *ptr_format; + int ret; + + /* Generate the format string. */ + ptr_format = stpcpy(new_format, format); + ret = sprintf(ptr_format, "%d", precision); + ptr_format += ret; + ptr_format = stpcpy(ptr_format, conversion); + + /* Call the float to string conversion function, e.g.: strfromd. */ + return FTOSTR(dest, size, new_format, value); +} + /* Compare KEY (a string, with the name of a function) with ULP (a pointer to a struct ulp_data structure), returning a value less than, equal to or greater than zero for use in bsearch. */ @@ -437,8 +460,8 @@ print_float (FLOAT f) else { char fstrn[FSTR_MAX], fstrx[FSTR_MAX]; - FTOSTR (fstrn, FSTR_MAX, "% .*" PRINTF_EXPR, TYPE_DECIMAL_DIG - 1, f); - FTOSTR (fstrx, FSTR_MAX, "% .*" PRINTF_XEXPR, TYPE_HEX_DIG - 1, f); + fmt_ftostr (fstrn, FSTR_MAX, "%.", PRINTF_EXPR, TYPE_DECIMAL_DIG - 1, f); + fmt_ftostr (fstrx, FSTR_MAX, "%.", PRINTF_XEXPR, TYPE_HEX_DIG - 1, f); printf ("%s %s\n", fstrn, fstrx); } } @@ -884,12 +907,12 @@ check_float_internal (const char *test_name, FLOAT computed, FLOAT expected, { char dstrn[FSTR_MAX], dstrx[FSTR_MAX]; char ustrn[FSTR_MAX], mustrn[FSTR_MAX]; - FTOSTR (dstrn, FSTR_MAX, "% .*" PRINTF_EXPR, - TYPE_DECIMAL_DIG - 1, diff); - FTOSTR (dstrx, FSTR_MAX, "% .*" PRINTF_XEXPR, - TYPE_HEX_DIG - 1, diff); - FTOSTR (ustrn, FSTR_MAX, "% .4" PRINTF_NEXPR, ulps); - FTOSTR (mustrn, FSTR_MAX, "% .4" PRINTF_NEXPR, max_ulp); + fmt_ftostr (dstrn, FSTR_MAX, "%.", PRINTF_EXPR, + TYPE_DECIMAL_DIG - 1, diff); + fmt_ftostr (dstrx, FSTR_MAX, "%.", PRINTF_XEXPR, + TYPE_HEX_DIG - 1, diff); + FTOSTR (ustrn, FSTR_MAX, "%.4" PRINTF_NEXPR, ulps); + FTOSTR (mustrn, FSTR_MAX, "%.4" PRINTF_NEXPR, max_ulp); printf (" difference: %s %s\n", dstrn, dstrx); printf (" ulp : %s\n", ustrn); printf (" max.ulp : %s\n", mustrn); diff --git a/math/test-double.h b/math/test-double.h index e172b8f..f8a57f9 100644 --- a/math/test-double.h +++ b/math/test-double.h @@ -26,5 +26,5 @@ #define LIT(x) (x) #define TYPE_STR "double" #define LITM(x) x -#define FTOSTR snprintf +#define FTOSTR strfromd #define snan_value_MACRO SNAN diff --git a/math/test-float.h b/math/test-float.h index ea096c8..6690966 100644 --- a/math/test-float.h +++ b/math/test-float.h @@ -27,5 +27,5 @@ #define LIT(x) (x ## f) /* Use the double variants of macro constants. */ #define LITM(x) x -#define FTOSTR snprintf +#define FTOSTR strfromf #define snan_value_MACRO SNANF diff --git a/math/test-ldouble.h b/math/test-ldouble.h index 62c9eb8..53c6bb3 100644 --- a/math/test-ldouble.h +++ b/math/test-ldouble.h @@ -18,13 +18,13 @@ #define FUNC(function) function##l #define FLOAT long double -#define PRINTF_EXPR "Le" -#define PRINTF_XEXPR "La" -#define PRINTF_NEXPR "Lf" +#define PRINTF_EXPR "e" +#define PRINTF_XEXPR "a" +#define PRINTF_NEXPR "f" #define BUILD_COMPLEX(real, imag) (CMPLXL ((real), (imag))) #define PREFIX LDBL #define TYPE_STR "ldouble" #define LIT(x) (x ## L) #define LITM(x) x ## l -#define FTOSTR snprintf +#define FTOSTR strfroml #define snan_value_MACRO SNANL