[libc++][format] Use to_chars internals.

[Mordante]

Summary

The external to_chars interface does some validations already done in
format. So instead of doing the same validations again, directly call
the internals of to_chars.

This has a small size benefit

  text	   data	    bss	    dec	    hex	filename
718800	  12472	    488	 731760	  b2a70	./formatter_int.libcxx.out-baseline
714915	  12472	    488	 727875	  b1b43	./formatter_int.libcxx.out

And a small performance benefit for simple formatting:

Comparing ./formatter_int.libcxx.out-baseline to ./formatter_int.libcxx.out
Benchmark                                                               Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------------
BM_Basic<uint32_t>                                                   -0.1139         -0.1125            61            54            61            54
BM_Basic<int32_t>                                                    -0.0265         -0.0250            62            60            62            60
BM_Basic<uint64_t>                                                   +0.0051         +0.0069            82            83            82            83
BM_Basic<int64_t>                                                    +0.0084         +0.0101            82            83            82            83
BM_BasicLow<__uint128_t>                                             -0.0588         -0.0572            87            82            87            82
BM_BasicLow<__int128_t>                                              -0.0548         -0.0532            88            83            88            83
BM_Basic<__uint128_t>                                                -0.0253         -0.0238           239           233           238           233
BM_Basic<__int128_t>                                                 -0.0346         -0.0331           222           214           222           214

[ldionne]

Thanks for the patch! I think the improvement is nice, but I have a few concerns about duplicating logic with to_chars.

libcxx/include/__format/formatter_integral.h

257–271

293

Why is this required? Wouldn't it be possible to instead call a single __to_chars_base function that contains this logic (but it would live inside the implementation of to_chars).

I think what I'm saying is that I don't understand why __to_chars_integral doesn't handle the various bases itself. IMO it should be possible to easily bypass the checks you're trying to avoid in to_chars without reimplementing this base dispatching.

311

Would it be possible to handle base 10 in __format_integer_base with if constexpr instead? I'm trying to find ways to make the switch below look more regular.

[Mordante]

Based on the review comments I think this as little merit to pursue.

libcxx/include/__format/formatter_integral.h

_LIBCPP_HIDE_FROM_ABI auto __format_char(
   return __formatter::__write(_VSTD::addressof(__c), _VSTD::addressof(__c) + 1, _VSTD::move(__out_it), __specs);
 }
 
 //
 // Integer
 //
 
 /** Wrapper around @ref to_chars, returning the output pointer. */
-template <integral _Tp>
-_LIBCPP_HIDE_FROM_ABI char* __to_buffer(char* __first, char* __last, _Tp __value, int __base) {
-  // TODO FMT Evaluate code overhead due to not calling the internal function
-  // directly. (Should be zero overhead.)
-  to_chars_result __r = _VSTD::to_chars(__first, __last, __value, __base);
+template <int _Base, unsigned_integral _Tp>
+_LIBCPP_HIDE_FROM_ABI char* __to_buffer(char* __first, char* __last, _Tp __value) {
+  to_chars_result __r = _VSTD::__to_chars_integral<_Base>(__first, __last, __value);
   _LIBCPP_ASSERT(__r.ec == errc(0), "Internal buffer too small");
   return __r.ptr;
 }
 
+#  ifndef _LIBCPP_HAS_NO_INT128
+template <int _Base>
+_LIBCPP_HIDE_FROM_ABI char* __to_buffer(char* __first, char* __last, __uint128_t __value) {
+  if (__value <= std::numeric_limits<uint64_t>::max())
+    return __to_buffer<_Base>(__first, __last, static_cast<uint64_t>(__value));
+
+  to_chars_result __r = _VSTD::__to_chars_integral<_Base>(__first, __last, __value);
+  _LIBCPP_ASSERT(__r.ec == errc(0), "Internal buffer too small");
+  return __r.ptr;
+}
+#  endif
+
 /**
  * Helper to determine the buffer size to output a integer in Base @em x.
  *
  * There are several overloads for the supported bases. The function uses the
  * base as template argument so it can be used in a constant expression.
  */
 template <unsigned_integral _Tp, size_t _Base>
 consteval size_t __buffer_size() noexcept
consteval size_t __buffer_size() noexcept
   requires(_Base == 16)
 {
   return numeric_limits<_Tp>::digits // The number of binary digits.
            / 4                       // Adjust to hexadecimal.
        + 2                           // Reserve space for the '0[Xx]' prefix.
        + 1;                          // Reserve space for the sign.
 }
 
-template <unsigned_integral _Tp, class _CharT>
+template <class _CharT>
 _LIBCPP_HIDE_FROM_ABI auto __format_integer(
-    _Tp __value,
-    auto& __ctx,
-    __format_spec::__parsed_specifications<_CharT> __specs,
-    bool __negative,
-    char* __begin,
-    char* __end,
-    const char* __prefix,
-    int __base) -> decltype(__ctx.out()) {
-  char* __first = __formatter::__insert_sign(__begin, __negative, __specs.__std_.__sign_);
-  if (__specs.__std_.__alternate_form_ && __prefix)
-    while (*__prefix)
-      *__first++ = *__prefix++;
-
-  char* __last = __formatter::__to_buffer(__first, __end, __value, __base);
-
+    char* __begin, char* __prefix, char* __end, auto& __ctx, __format_spec::__parsed_specifications<_CharT> __specs)
+    -> decltype(__ctx.out()) {
 #  ifndef _LIBCPP_HAS_NO_LOCALIZATION
   if (__specs.__std_.__locale_specific_form_) {
     const auto& __np  = use_facet<numpunct<_CharT>>(__ctx.locale());
     string __grouping = __np.grouping();
-    ptrdiff_t __size  = __last - __first;
+    ptrdiff_t __size  = __end - __prefix;
     // Writing the grouped form has more overhead than the normal output
     // routines. If there will be no separators written the locale-specific
     // form is identical to the normal routine. Test whether to grouped form
     // is required.
     if (!__grouping.empty() && __size > __grouping[0])
       return __formatter::__write_using_decimal_separators(
           __ctx.out(),
           __begin,
-          __first,
-          __last,
+          __prefix,
+          __end,
           __formatter::__determine_grouping(__size, __grouping),
           __np.thousands_sep(),
           __specs);
   }
 #  endif
   auto __out_it = __ctx.out();
-  if (__specs.__alignment_ != __format_spec::__alignment::__zero_padding)
-    __first = __begin;
-  else {
+  if (__specs.__alignment_ == __format_spec::__alignment::__zero_padding) {
     // __buf contains [sign][prefix]data
     //                              ^ location of __first
     // The zero padding is done like:
     // - Write [sign][prefix]
     // - Write data right aligned with '0' as fill character.
-    __out_it             = _VSTD::copy(__begin, __first, _VSTD::move(__out_it));
+    __out_it             = _VSTD::copy(__begin, __prefix, _VSTD::move(__out_it));
     __specs.__alignment_ = __format_spec::__alignment::__right;
     __specs.__fill_      = _CharT('0');
-    int32_t __size       = __first - __begin;
+    int32_t __size       = __prefix - __begin;
 
     __specs.__width_ -= _VSTD::min(__size, __specs.__width_);
+    __begin = __prefix;
   }
 
   if (__specs.__std_.__type_ != __format_spec::__type::__hexadecimal_upper_case) [[likely]]
-    return __formatter::__write(__first, __last, __ctx.out(), __specs);
+    return __formatter::__write(__begin, __end, __ctx.out(), __specs);
+
+  return __formatter::__write_transformed(__begin, __end, __ctx.out(), __specs, __formatter::__hex_to_upper);
+}
+
+/// Writes the base prefix in the buffer according to [tab:format.type.int]
+template <int _Base, unsigned_integral _Tp>
+char* __set_prefix(char* __begin, _Tp __value, bool __upper_case) {
+  *__begin = '0';
+  if constexpr (_Base == 2) {
+    *(__begin + 1) = __upper_case ? 'B' : 'b';
+    return __begin + 2;
+  } else if constexpr (_Base == 8) {
+    return __value == 0 ? __begin : __begin + 1;
+  } else if constexpr (_Base == 16) {
+    *(__begin + 1) = __upper_case ? 'X' : 'x';
+    return __begin + 2;
+  } else {
+    static_assert(sizeof(_Tp) == 0, "unsupported base");
+  }
+}
+
+template <int _Base, unsigned_integral _Tp, class _CharT>
+_LIBCPP_HIDE_FROM_ABI auto
+__format_integer_base(_Tp __value, auto& __ctx, __format_spec::__parsed_specifications<_CharT> __specs, bool __negative)
+    -> decltype(__ctx.out()) {
+  array<char, __formatter::__buffer_size<decltype(__value), _Base>()> __array;
+
+  char* __prefix = __formatter::__insert_sign(__array.begin(), __negative, __specs.__std_.__sign_);
+  if (__specs.__std_.__alternate_form_) {
+    bool __upper_case =
+        __specs.__std_.__type_ == __format_spec::__type::__binary_upper_case ||
+        __specs.__std_.__type_ == __format_spec::__type::__hexadecimal_upper_case;
+
+    __prefix = __set_prefix<_Base>(__prefix, __value, __upper_case);
+  }
+
+  char* __last = __to_buffer<_Base>(__prefix, __array.end(), __value);
+  return __formatter::__format_integer(__array.begin(), __prefix, __last, __ctx, __specs);
+}
+
+template <unsigned_integral _Tp>
+char* __to_chars_base_10(char* __first, _Tp __value) {
+  if constexpr (sizeof(_Tp) <= sizeof(uint32_t))
+    return __itoa::__base_10_u32(__first, static_cast<uint32_t>(__value));
+  else if constexpr (sizeof(_Tp) == sizeof(uint64_t))
+    return __itoa::__base_10_u64(__first, static_cast<uint64_t>(__value));
+#  ifndef _LIBCPP_HAS_NO_INT128
+  else if constexpr (sizeof(_Tp) == sizeof(__uint128_t)) {
+    if (__value <= std::numeric_limits<uint64_t>::max())
+      return __itoa::__base_10_u64(__first, static_cast<uint64_t>(__value));
+    else
+      return __itoa::__base_10_u128(__first, __value);
+  }
+#  endif
+  else
+    static_assert(sizeof(_Tp) == 0, "unsupported unsigned integral type");
+}
+
+template <unsigned_integral _Tp, class _CharT>
+_LIBCPP_HIDE_FROM_ABI auto __format_integer_base_10(
+    _Tp __value, auto& __ctx, __format_spec::__parsed_specifications<_CharT> __specs, bool __negative)
+    -> decltype(__ctx.out()) {
+  array<char, __formatter::__buffer_size<decltype(__value), 10>()> __array;
+
+  char* __prefix = __formatter::__insert_sign(__array.begin(), __negative, __specs.__std_.__sign_);
+  char* __last   = __formatter::__to_chars_base_10(__prefix, __value);
 
-  return __formatter::__write_transformed(__first, __last, __ctx.out(), __specs, __formatter::__hex_to_upper);
+  return __formatter::__format_integer(__array.begin(), __prefix, __last, __ctx, __specs);
 }
 
 template <unsigned_integral _Tp, class _CharT>
 _LIBCPP_HIDE_FROM_ABI auto __format_integer(
     _Tp __value, auto& __ctx, __format_spec::__parsed_specifications<_CharT> __specs, bool __negative = false)
     -> decltype(__ctx.out()) {
   switch (__specs.__std_.__type_) {
-  case __format_spec::__type::__binary_lower_case: {
-    array<char, __formatter::__buffer_size<decltype(__value), 2>()> __array;
-    return __formatter::__format_integer(__value, __ctx, __specs, __negative, __array.begin(), __array.end(), "0b", 2);
-  }
-  case __format_spec::__type::__binary_upper_case: {
-    array<char, __formatter::__buffer_size<decltype(__value), 2>()> __array;
-    return __formatter::__format_integer(__value, __ctx, __specs, __negative, __array.begin(), __array.end(), "0B", 2);
-  }
-  case __format_spec::__type::__octal: {
-    // Octal is special; if __value == 0 there's no prefix.
-    array<char, __formatter::__buffer_size<decltype(__value), 8>()> __array;
-    return __formatter::__format_integer(
-        __value, __ctx, __specs, __negative, __array.begin(), __array.end(), __value != 0 ? "0" : nullptr, 8);
-  }
+  case __format_spec::__type::__binary_lower_case:
+  case __format_spec::__type::__binary_upper_case:
+    return __formatter::__format_integer_base<2>(__value, __ctx, __specs, __negative);
+
+  case __format_spec::__type::__octal:
+    return __formatter::__format_integer_base<8>(__value, __ctx, __specs, __negative);
+
   case __format_spec::__type::__default:
-  case __format_spec::__type::__decimal: {
-    array<char, __formatter::__buffer_size<decltype(__value), 10>()> __array;
-    return __formatter::__format_integer(
-        __value, __ctx, __specs, __negative, __array.begin(), __array.end(), nullptr, 10);
-  }
-  case __format_spec::__type::__hexadecimal_lower_case: {
-    array<char, __formatter::__buffer_size<decltype(__value), 16>()> __array;
-    return __formatter::__format_integer(__value, __ctx, __specs, __negative, __array.begin(), __array.end(), "0x", 16);
-  }
-  case __format_spec::__type::__hexadecimal_upper_case: {
-    array<char, __formatter::__buffer_size<decltype(__value), 16>()> __array;
-    return __formatter::__format_integer(__value, __ctx, __specs, __negative, __array.begin(), __array.end(), "0X", 16);
-  }
+  case __format_spec::__type::__decimal:
+    return __formatter::__format_integer_base_10(__value, __ctx, __specs, __negative);
+
+  case __format_spec::__type::__hexadecimal_lower_case:
+  case __format_spec::__type::__hexadecimal_upper_case:
+    return __formatter::__format_integer_base<16>(__value, __ctx, __specs, __negative);
+
   default:
     _LIBCPP_ASSERT(false, "The parse function should have validated the type");
     __libcpp_unreachable();
   }
 }
 
 template <signed_integral _Tp, class _CharT>
 _LIBCPP_HIDE_FROM_ABI auto