llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) <details> <summary>Changes</summary> The internal format buffer code shipped with LLVM 19 is no longer used and removed. This also updates parts of the documentation to reflect its proper state. --- Patch is 21.99 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101876.diff 9 Files Affected: - (modified) libcxx/include/__format/buffer.h (+32-286) - (modified) libcxx/test/libcxx/transitive_includes/cxx03.csv (-1) - (modified) libcxx/test/libcxx/transitive_includes/cxx11.csv (-1) - (modified) libcxx/test/libcxx/transitive_includes/cxx14.csv (-1) - (modified) libcxx/test/libcxx/transitive_includes/cxx17.csv (-1) - (modified) libcxx/test/libcxx/transitive_includes/cxx20.csv (-2) - (modified) libcxx/test/libcxx/transitive_includes/cxx23.csv (-2) - (modified) libcxx/test/libcxx/transitive_includes/cxx26.csv (-2) - (modified) libcxx/test/std/utilities/format/format.functions/format_tests.h (+1-1) ``````````diff diff --git a/libcxx/include/__format/buffer.h b/libcxx/include/__format/buffer.h index ca2334f93fd04..b0722535c6d87 100644 --- a/libcxx/include/__format/buffer.h +++ b/libcxx/include/__format/buffer.h @@ -39,7 +39,6 @@ #include <__type_traits/conditional.h> #include <__utility/exception_guard.h> #include <__utility/move.h> -#include <climits> // LLVM-20 remove #include <cstddef> #include <stdexcept> #include <string_view> @@ -63,7 +62,7 @@ class _LIBCPP_HIDE_FROM_ABI __max_output_size { [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit __max_output_size(size_t __max_size) : __max_size_{__max_size} {} // This function adjusts the size of a (bulk) write operations. It ensures the - // number of code units written by a __output_buffer never exceed + // number of code units written by a __output_buffer never exceeds // __max_size_ code units. [[nodiscard]] _LIBCPP_HIDE_FROM_ABI size_t __write_request(size_t __code_units) { size_t __result = @@ -87,26 +86,7 @@ class _LIBCPP_HIDE_FROM_ABI __max_output_size { /// type-erasure for the formatting functions. This reduces the number to /// template instantiations. /// -/// The design of the class is being changed to improve performance and do some -/// code cleanups. -/// The original design (as shipped up to LLVM-19) uses the following design: -/// - There is an external object that connects the buffer to the output. -/// - The class constructor stores a function pointer to a grow function and a -/// type-erased pointer to the object that does the grow. -/// - When writing data to the buffer would exceed the external buffer's -/// capacity it requests the external buffer to flush its contents. -/// -/// The new design tries to solve some issues with the current design: -/// - The buffer used is a fixed-size buffer, benchmarking shows that using a -/// dynamic allocated buffer has performance benefits. -/// - Implementing P3107R5 "Permit an efficient implementation of std::print" -/// is not trivial with the current buffers. Using the code from this series -/// makes it trivial. -/// -/// This class is ABI-tagged, still the new design does not change the size of -/// objects of this class. -/// -/// The new design is the following. +/// The design is the following: /// - There is an external object that connects the buffer to the output. /// - This buffer object: /// - inherits publicly from this class. @@ -206,37 +186,19 @@ class _LIBCPP_TEMPLATE_VIS __output_buffer { using value_type = _CharT; using __prepare_write_type = void (*)(__output_buffer<_CharT>&, size_t); - template <class _Tp> // Deprecated LLVM-19 function. - _LIBCPP_HIDE_FROM_ABI explicit __output_buffer(_CharT* __ptr, size_t __capacity, _Tp* __obj) - : __ptr_(__ptr), - __capacity_(__capacity), - __flush_([](_CharT* __p, size_t __n, void* __o) { static_cast<_Tp*>(__o)->__flush(__p, __n); }), - __data_{.__version_llvm_20__ = false, .__obj_ = reinterpret_cast<uintptr_t>(__obj) >> 1} {} - - // New LLVM-20 function. [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit __output_buffer(_CharT* __ptr, size_t __capacity, __prepare_write_type __prepare_write) : __output_buffer{__ptr, __capacity, __prepare_write, nullptr} {} - // New LLVM-20 function. [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit __output_buffer( _CharT* __ptr, size_t __capacity, __prepare_write_type __prepare_write, __max_output_size* __max_output_size) : __ptr_(__ptr), __capacity_(__capacity), __prepare_write_(__prepare_write), - __data_{.__version_llvm_20_ = true, .__max_output_size_ = reinterpret_cast<uintptr_t>(__max_output_size) >> 1} { - } - - // Deprecated LLVM-19 function. - _LIBCPP_HIDE_FROM_ABI void __reset(_CharT* __ptr, size_t __capacity) { - __ptr_ = __ptr; - __capacity_ = __capacity; - } + __max_output_size_(__max_output_size) {} - // New LLVM-20 function. _LIBCPP_HIDE_FROM_ABI void __buffer_flused() { __size_ = 0; } - // New LLVM-20 function. _LIBCPP_HIDE_FROM_ABI void __buffer_moved(_CharT* __ptr, size_t __capacity) { __ptr_ = __ptr; __capacity_ = __capacity; @@ -246,16 +208,18 @@ class _LIBCPP_TEMPLATE_VIS __output_buffer { // Used in std::back_insert_iterator. _LIBCPP_HIDE_FROM_ABI void push_back(_CharT __c) { - if (__data_.__version_llvm_20_ && __data_.__max_output_size_ && - reinterpret_cast<__max_output_size*>(__data_.__max_output_size_ << 1)->__write_request(1) == 0) + if (__max_output_size_ && __max_output_size_->__write_request(1) == 0) return; + _LIBCPP_ASSERT_INTERNAL( + __ptr_ && __size_ < __capacity_ && __available() >= 1, "attempted to write outside the buffer"); + __ptr_[__size_++] = __c; // Profiling showed flushing after adding is more efficient than flushing // when entering the function. if (__size_ == __capacity_) - __flush(0); + __prepare_write(0); } /// Copies the input __str to the buffer. @@ -276,30 +240,20 @@ class _LIBCPP_TEMPLATE_VIS __output_buffer { // upper case. For integral these strings are short. // TODO FMT Look at the improvements above. size_t __n = __str.size(); - if (__data_.__version_llvm_20_ && __data_.__max_output_size_) { - __n = reinterpret_cast<__max_output_size*>(__data_.__max_output_size_ << 1)->__write_request(__n); + if (__max_output_size_) { + __n = __max_output_size_->__write_request(__n); if (__n == 0) return; } - __flush_on_overflow(__n); - if (__n < __capacity_) { // push_back requires the buffer to have room for at least one character (so use <). - std::copy_n(__str.data(), __n, std::addressof(__ptr_[__size_])); - __size_ += __n; - return; - } - - // The output doesn't fit in the internal buffer. - // Copy the data in "__capacity_" sized chunks. - _LIBCPP_ASSERT_INTERNAL(__size_ == 0, "the buffer should be flushed by __flush_on_overflow"); const _InCharT* __first = __str.data(); do { - size_t __chunk = std::min(__n, __capacity_ - __size_); + __prepare_write(__n); + size_t __chunk = std::min(__n, __available()); std::copy_n(__first, __chunk, std::addressof(__ptr_[__size_])); - __size_ = __chunk; + __size_ += __chunk; __first += __chunk; __n -= __chunk; - __flush(__n); } while (__n); } @@ -313,70 +267,39 @@ class _LIBCPP_TEMPLATE_VIS __output_buffer { _LIBCPP_ASSERT_INTERNAL(__first <= __last, "not a valid range"); size_t __n = static_cast<size_t>(__last - __first); - if (__data_.__version_llvm_20_ && __data_.__max_output_size_) { - __n = reinterpret_cast<__max_output_size*>(__data_.__max_output_size_ << 1)->__write_request(__n); + if (__max_output_size_) { + __n = __max_output_size_->__write_request(__n); if (__n == 0) return; } - __flush_on_overflow(__n); - if (__n < __capacity_) { // push_back requires the buffer to have room for at least one character (so use <). - std::transform(__first, __last, std::addressof(__ptr_[__size_]), std::move(__operation)); - __size_ += __n; - return; - } - - // The output doesn't fit in the internal buffer. - // Transform the data in "__capacity_" sized chunks. - _LIBCPP_ASSERT_INTERNAL(__size_ == 0, "the buffer should be flushed by __flush_on_overflow"); do { - size_t __chunk = std::min(__n, __capacity_ - __size_); + __prepare_write(__n); + size_t __chunk = std::min(__n, __available()); std::transform(__first, __first + __chunk, std::addressof(__ptr_[__size_]), __operation); - __size_ = __chunk; + __size_ += __chunk; __first += __chunk; __n -= __chunk; - __flush(__n); } while (__n); } /// A \c fill_n wrapper. _LIBCPP_HIDE_FROM_ABI void __fill(size_t __n, _CharT __value) { - if (__data_.__version_llvm_20_ && __data_.__max_output_size_) { - __n = reinterpret_cast<__max_output_size*>(__data_.__max_output_size_ << 1)->__write_request(__n); + if (__max_output_size_) { + __n = __max_output_size_->__write_request(__n); if (__n == 0) return; } - __flush_on_overflow(__n); - if (__n < __capacity_) { // push_back requires the buffer to have room for at least one character (so use <). - std::fill_n(std::addressof(__ptr_[__size_]), __n, __value); - __size_ += __n; - return; - } - - // The output doesn't fit in the internal buffer. - // Fill the buffer in "__capacity_" sized chunks. - _LIBCPP_ASSERT_INTERNAL(__size_ == 0, "the buffer should be flushed by __flush_on_overflow"); do { - size_t __chunk = std::min(__n, __capacity_); + __prepare_write(__n); + size_t __chunk = std::min(__n, __available()); std::fill_n(std::addressof(__ptr_[__size_]), __chunk, __value); - __size_ = __chunk; + __size_ += __chunk; __n -= __chunk; - __flush(__n); } while (__n); } - _LIBCPP_HIDE_FROM_ABI void __flush(size_t __size_hint) { - if (!__data_.__version_llvm_20_) { - // LLVM-19 code path - __flush_(__ptr_, __size_, reinterpret_cast<void*>(__data_.__obj_ << 1)); - __size_ = 0; - } else { - // LLVM-20 code path - __prepare_write_(*this, __size_hint + 1); // + 1 to always have space for the next time - } - } - [[nodiscard]] _LIBCPP_HIDE_FROM_ABI size_t __capacity() const { return __capacity_; } [[nodiscard]] _LIBCPP_HIDE_FROM_ABI size_t __size() const { return __size_; } @@ -384,94 +307,19 @@ class _LIBCPP_TEMPLATE_VIS __output_buffer { _CharT* __ptr_; size_t __capacity_; size_t __size_{0}; - union { - // LLVM-19 member - void (*__flush_)(_CharT*, size_t, void*); - // LLVM-20 member - void (*__prepare_write_)(__output_buffer<_CharT>&, size_t); - }; - static_assert(sizeof(__flush_) == sizeof(__prepare_write_), "The union is an ABI break."); - static_assert(alignof(decltype(__flush_)) == alignof(decltype(__prepare_write_)), "The union is an ABI break."); - // Note this code is quite ugly, but it can cleaned up once the LLVM-19 parts - // of the code are removed. - union { - struct { - uintptr_t __version_llvm_20__ : 1; - uintptr_t __obj_ : CHAR_BIT * sizeof(void*) - 1; - }; - struct { - uintptr_t __version_llvm_20_ : 1; - uintptr_t __max_output_size_ : CHAR_BIT * sizeof(__max_output_size*) - 1; - }; - } __data_; - static_assert(sizeof(__data_) == sizeof(void*), "The struct is an ABI break."); - static_assert(alignof(decltype(__data_)) == alignof(void*), "The struct is an ABI break."); - - /// Flushes the buffer when the output operation would overflow the buffer. - /// - /// A simple approach for the overflow detection would be something along the - /// lines: - /// \code - /// // The internal buffer is large enough. - /// if (__n <= __capacity_) { - /// // Flush when we really would overflow. - /// if (__size_ + __n >= __capacity_) - /// __flush(); - /// ... - /// } - /// \endcode - /// - /// This approach works for all cases but one: - /// A __format_to_n_buffer_base where \ref __enable_direct_output is true. - /// In that case the \ref __capacity_ of the buffer changes during the first - /// \ref __flush. During that operation the output buffer switches from its - /// __writer_ to its __storage_. The \ref __capacity_ of the former depends - /// on the value of n, of the latter is a fixed size. For example: - /// - a format_to_n call with a 10'000 char buffer, - /// - the buffer is filled with 9'500 chars, - /// - adding 1'000 elements would overflow the buffer so the buffer gets - /// changed and the \ref __capacity_ decreases from 10'000 to - /// __buffer_size (256 at the time of writing). - /// - /// This means that the \ref __flush for this class may need to copy a part of - /// the internal buffer to the proper output. In this example there will be - /// 500 characters that need this copy operation. - /// - /// Note it would be more efficient to write 500 chars directly and then swap - /// the buffers. This would make the code more complex and \ref format_to_n is - /// not the most common use case. Therefore the optimization isn't done. - _LIBCPP_HIDE_FROM_ABI void __flush_on_overflow(size_t __n) { - __n += __size_; - if (__n >= __capacity_) - __flush(__n - __capacity_ + 1); - } -}; - -// ***** ***** ***** LLVM-19 classes ***** ***** ***** - -/// A storage using an internal buffer. -/// -/// This storage is used when writing a single element to the output iterator -/// is expensive. -template <__fmt_char_type _CharT> -class _LIBCPP_TEMPLATE_VIS __internal_storage { -public: - _LIBCPP_HIDE_FROM_ABI _CharT* __begin() { return __buffer_; } + void (*__prepare_write_)(__output_buffer<_CharT>&, size_t); + __max_output_size* __max_output_size_; - static constexpr size_t __buffer_size = 256 / sizeof(_CharT); + [[nodiscard]] _LIBCPP_HIDE_FROM_ABI size_t __available() const { return __capacity_ - __size_; } -private: - _CharT __buffer_[__buffer_size]; + _LIBCPP_HIDE_FROM_ABI void __prepare_write(size_t __code_units) { + // Always have space for one additional code unit. This is a precondition of the push_back function. + __code_units += 1; + if (__available() < __code_units) + __prepare_write_(*this, __code_units + 1); + } }; -/// A storage writing directly to the storage. -/// -/// This requires the storage to be a contiguous buffer of \a _CharT. -/// Since the output is directly written to the underlying storage this class -/// is just an empty class. -template <__fmt_char_type _CharT> -class _LIBCPP_TEMPLATE_VIS __direct_storage {}; - template <class _OutIt, class _CharT> concept __enable_direct_output = __fmt_char_type<_CharT> && @@ -480,40 +328,6 @@ concept __enable_direct_output = // `#ifdef`. || same_as<_OutIt, __wrap_iter<_CharT*>>); -/// Write policy for directly writing to the underlying output. -template <class _OutIt, __fmt_char_type _CharT> -class _LIBCPP_TEMPLATE_VIS __writer_direct { -public: - _LIBCPP_HIDE_FROM_ABI explicit __writer_direct(_OutIt __out_it) : __out_it_(__out_it) {} - - _LIBCPP_HIDE_FROM_ABI _OutIt __out_it() { return __out_it_; } - - _LIBCPP_HIDE_FROM_ABI void __flush(_CharT*, size_t __n) { - // _OutIt can be a __wrap_iter<CharT*>. Therefore the original iterator - // is adjusted. - __out_it_ += __n; - } - -private: - _OutIt __out_it_; -}; - -/// Write policy for copying the buffer to the output. -template <class _OutIt, __fmt_char_type _CharT> -class _LIBCPP_TEMPLATE_VIS __writer_iterator { -public: - _LIBCPP_HIDE_FROM_ABI explicit __writer_iterator(_OutIt __out_it) : __out_it_{std::move(__out_it)} {} - - _LIBCPP_HIDE_FROM_ABI _OutIt __out_it() && { return std::move(__out_it_); } - - _LIBCPP_HIDE_FROM_ABI void __flush(_CharT* __ptr, size_t __n) { - __out_it_ = std::ranges::copy_n(__ptr, __n, std::move(__out_it_)).out; - } - -private: - _OutIt __out_it_; -}; - /// Concept to see whether a \a _Container is insertable. /// /// The concept is used to validate whether multiple calls to a @@ -539,72 +353,6 @@ struct _LIBCPP_TEMPLATE_VIS __back_insert_iterator_container<back_insert_iterato using type = _Container; }; -/// Write policy for inserting the buffer in a container. -template <class _Container> -class _LIBCPP_TEMPLATE_VIS __writer_container { -public: - using _CharT = typename _Container::value_type; - - _LIBCPP_HIDE_FROM_ABI explicit __writer_container(back_insert_iterator<_Container> __out_it) - : __container_{__out_it.__get_container()} {} - - _LIBCPP_HIDE_FROM_ABI auto __out_it() { return std::back_inserter(*__container_); } - - _LIBCPP_HIDE_FROM_ABI void __flush(_CharT* __ptr, size_t __n) { - __container_->insert(__container_->end(), __ptr, __ptr + __n); - } - -private: - _Container* __container_; -}; - -/// Selects the type of the writer used for the output iterator. -template <class _OutIt, class _CharT> -class _LIBCPP_TEMPLATE_VIS __writer_selector { - using _Container = typename __back_insert_iterator_container<_OutIt>::type; - -public: - using type = - conditional_t<!same_as<_Container, void>, - __writer_container<_Container>, - conditional_t<__enable_direct_output<_OutIt, _CharT>, - __writer_direct<_OutIt, _CharT>, - __writer_iterator<_OutIt, _CharT>>>; -}; - -/// The generic formatting buffer. -template <class _OutIt, __fmt_char_type _CharT> - requires(output_iterator<_OutIt, const _CharT&>) -class _LIBCPP_TEMPLATE_VIS __format_buffer { - using _Storage = - conditional_t<__enable_direct_output<_OutIt, _CharT>, __direct_storage<_CharT>, __internal_storage<_CharT>>; - -public: - _LIBCPP_HIDE_FROM_ABI explicit __format_buffer(_OutIt __out_it) - requires(same_as<_Storage, __internal_storage<_CharT>>) - : __output_(__storage_.__begin(), __storage_.__buffer_size, this), __writer_(std::move(__out_it)) {} - - _LIBCPP_HIDE_FROM_ABI explicit __format_buffer(_OutIt __out_it) - requires(same_as<_Storage, __direct_storage<_CharT>>) - : __output_(std::__unwrap_iter(__out_it), size_t(-1), this), __writer_(std::move(__out_it)) {} - - _LIBCPP_HIDE_FROM_ABI auto __make_output_iterator() { return __output_.__make_output_iterator(); } - - _LIBCPP_HIDE_FROM_ABI void __flush(_CharT* __ptr, size_t __n) { __writer_.__flush(__ptr, __n); } - - _LIBCPP_HIDE_FROM_ABI _OutIt __out_it() && { - __output_.__flush(0); - return std::move(__writer_).__out_it(); - } - -private: - _LIBCPP_NO_UNIQUE_ADDRESS _Storage __storage_; - __output_buffer<_CharT> __output_; - typename __writer_selector<_OutIt, _CharT>::type __writer_; -}; - -// ***** ***** ***** LLVM-20 classes ***** ***** ***** - // A dynamically growing buffer. template <__fmt_char_type _CharT> class _LIBCPP_TEMPLATE_VIS __allocating_buffer : public __output_buffer<_CharT> { @@ -824,8 +572,6 @@ class _LIBCPP_TEMPLATE_VIS __formatted_size_buffer : private __output_buffer<_Ch } }; -// ***** ***** ***** LLVM-19 and LLVM-20 class ***** ***** ***** - // A dynamically growing buffer intended to be used for retargeting a context. // // P2286 Formatting ranges adds range formatting support. It allows the user to diff --git a/libcxx/test/libcxx/transitive_includes/cxx03.csv b/libcxx/test/libcxx/transitive_includes/cxx03.csv index f6e7db17f413f..622fced5ffa40 100644 --- a/libcxx/test/libcxx/transitive_includes/cxx03.csv +++ b/libcxx/test/libcxx/transitive_includes/cxx03.csv @@ -860,7 +860,6 @@ thread atomic thread cctype thread cerrno thread chrono -thread climits thread clocale thread compare thread cstddef diff --git a/libcxx/test/libcxx/transitive_includes/cxx11.csv b/libcxx/test/libcxx/transitive_includes/cxx11.csv index 752fea058e63b..11c3c1322c406 100644 --- a/libcxx/test/libcxx/transitive_includes/cxx11.csv +++ b/libcxx/test/libcxx/transitive_includes/cxx11.csv @@ -867,7 +867,6 @@ thread atomic thread cctype thread cerrno thread chrono -thread climits thread clocale thread compare thread cstddef diff --git a/libcxx/test/libcxx/transitive_includes/cxx14.csv b/libcxx/test/libcxx/transitive_includes/cxx14.csv index 010f7e2fb82e9..666d5c3896467 100644 --- a/libcxx/test/libcxx/transitive_includes/cxx14.csv +++ b/libcxx/test/libcxx/transitive_includes/cxx14.csv @@ -870,7 +870,6 @@ thread atomic thread cctype thread cerrno thread chrono -thread climits thread clocale thread compare thread cstddef diff --git a/libcxx/test/libcxx/transitive_includes/cxx17.csv b/libcxx/test/libcxx/transitive_includes/cxx17.csv index 64c2db3eef6f9..3a3aa5a894473 100644 --- a/libcxx/test/libcxx/transitive_includes/cxx17.csv +++ b/libcxx/test/libcxx/transitive_includes/cxx17.csv @@ -881,7 +881,6 @@ thread atomic thread cctype thread cerrno thread chrono -thread climits thread clocale thread compare thread cstddef diff --git a/libcxx/test/libcxx/transitive_includes/cxx20.csv b/libcxx/test/libcxx/transitive_includes/cxx20.csv index a7ea7f8dddbea..982c2013e3417 100644 --- a/libcxx/test/libcxx/transitive_includes/cxx20.csv +++ b/libcxx/test/libcxx/transitive_includes/cxx20.csv @@ -276,7 +276,6 @@ filesystem version format array format cctype format cerrno -format climits format clocale f... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/101876 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
