diff mbox

Replace use of snprintf with strfrom in libm tests

Message ID 1481563942-19034-1-git-send-email-gftg@linux.vnet.ibm.com
State New
Headers show

Commit Message

Gabriel F. T. Gomes Dec. 12, 2016, 5:32 p.m. UTC
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}.

2016-11-08  Gabriel F. T. Gomes  <gftg@linux.vnet.ibm.com>

	* math/libm-test.inc (fmt_ftostr): New function.
	(print_float): Use fmt_ftostr instead of FTOSTR.
	(check_float_internal): Likewise.
	* math/test-double.h (FTOSTR): Define to strfromd.
	* math/test-float.h (FTOSTR): Define to strfromf.
	* math/test-ldouble.h (FTOSTR): Define to strfroml.
	(PRINTF_EXPR): Remove length modifier ('L') from format string.
	(PRINTF_XEXPR): Likewise.
	(PRINTF_NEXPR): Likewise.
---
 math/libm-test.inc  | 39 +++++++++++++++++++++++++++++++--------
 math/test-double.h  |  2 +-
 math/test-float.h   |  2 +-
 math/test-ldouble.h |  8 ++++----
 4 files changed, 37 insertions(+), 14 deletions(-)

Comments

Joseph Myers Dec. 13, 2016, 6:56 p.m. UTC | #1
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 mbox

Patch

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