Re: [PATCH v3 03/11] Refactor *cvt functions implementation (1-4/5)

[Paul E Murphy <murphyp at linux dot ibm dot com>]

On 12/3/19 11:05 AM, Gabriel F. T. Gomes wrote:
> From: "Gabriel F. T. Gomes" <gabrielftg@linux.ibm.com>
>
> Changes since v2:
>
>    - Squashed patches 1-4 and trivial bits of patch 5.
>    - Replaced http with https (like the rest of glibc).
>    - Updated copyright years.
>    - Removed refactoring noise reported by Paul Murphy.
>
> No changes since v1.
>
> This patch is to be squashed with the other n/5 refactoring patches.
>
> Below, the commit message to be used after the squashing...
>
> -- 8< --
> This patch refactors the *cvt functions implementation in a way that
> makes it easier to re-use them for implementing the IEEE long double on
> powerpc64le.  By splitting the implementation per se in one file
> (efgcvt-template.c) and the alias definitions in others (e.g. efgcvt.c),
> the new code makes it easier to define new function names, such as
> __qecvtieee128.
>
> Tested for powerpc64le and with build-many-glibcs.

Is it still your intent to squash the two remaining commits?  This 
refactoring patch is still missing some changes included in the 
followup.  I still think two self-sufficient patches read better than 
squashing these two.  Otherwise, this patch is much improved over 4.

...

> --- a/misc/qefgcvt_r.c
> +++ b/misc/qefgcvt_r.c
> @@ -17,41 +17,19 @@
>      License along with the GNU C Library; if not, see
>      <https://www.gnu.org/licenses/>.  */

This seems to be missing at least the following:

#include <efgcvt-ldbl-macros.h>
#include <efgcvt_r-template.c>

Similarly for some others.

> -#include <float.h>
> -
> -#define FLOAT_TYPE long double
> -#define FUNC_PREFIX q
> -#define FLOAT_FMT_FLAG "L"
> -#define FLOAT_NAME_EXT l
> -#define FLOAT_MIN_10_EXP LDBL_MIN_10_EXP
> -#if LDBL_MANT_DIG == 64
> -# define NDIGIT_MAX 21
> -#elif LDBL_MANT_DIG == 53
> -# define NDIGIT_MAX 17
> -#elif LDBL_MANT_DIG == 113
> -# define NDIGIT_MAX 36
> -#elif LDBL_MANT_DIG == 106
> -# define NDIGIT_MAX 34
> -#elif LDBL_MANT_DIG == 56
> -# define NDIGIT_MAX 18
> -#else
> -/* See IEEE 854 5.6, table 2 for this formula.  Unfortunately we need a
> -   compile time constant here, so we cannot use it.  */
> -# error "NDIGIT_MAX must be precomputed"
> -# define NDIGIT_MAX (lrint (ceil (M_LN2 / M_LN10 * LDBL_MANT_DIG + 1.0)))
> -#endif
> -#if LDBL_MIN_10_EXP == -37
> -# define FLOAT_MIN_10_NORM	1.0e-37L
> -#elif LDBL_MIN_10_EXP == -291
> -# define FLOAT_MIN_10_NORM	1.0e-291L
> -#elif LDBL_MIN_10_EXP == -307
> -# define FLOAT_MIN_10_NORM	1.0e-307L
> -#elif LDBL_MIN_10_EXP == -4931
> -# define FLOAT_MIN_10_NORM	1.0e-4931L
> +#if LONG_DOUBLE_COMPAT (libc, GLIBC_2_0)
> +# define cvt_symbol(symbol) \
> +  cvt_symbol_1 (libc, __APPEND (FUNC_PREFIX, symbol), \
> +	      APPEND (FUNC_PREFIX, symbol), GLIBC_2_4)
> +# define cvt_symbol_1(lib, local, symbol, version) \
> +  libc_hidden_def (local) \
> +  versioned_symbol (lib, local, symbol, version)
>   #else
> -/* libc can't depend on libm.  */
> -# error "FLOAT_MIN_10_NORM must be precomputed"
> -# define FLOAT_MIN_10_NORM	exp10l (LDBL_MIN_10_EXP)
> +# define cvt_symbol(symbol) \
> +  cvt_symbol_1 (__APPEND (FUNC_PREFIX, symbol), APPEND (FUNC_PREFIX, symbol))
> +#  define cvt_symbol_1(local, symbol) \
> +  libc_hidden_def (local) \
> +  weak_alias (local, symbol)
>   #endif
> -
> -#include "efgcvt_r.c"
> +cvt_symbol(fcvt_r);
> +cvt_symbol(ecvt_r);