On Tue, 26 Nov 2024 at 12:08, Jan Hubicka <hubi...@ucw.cz> wrote:
>
> > > diff --git a/libstdc++-v3/include/bits/deque.tcc
> > > b/libstdc++-v3/include/bits/deque.tcc
> > > index deb010a0ebb..e56cd0b9319 100644
> > > --- a/libstdc++-v3/include/bits/deque.tcc
> > > +++ b/libstdc++-v3/include/bits/deque.tcc
> > > @@ -955,6 +955,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> > > size_type __new_map_size = this->_M_impl._M_map_size
> > > + std::max(this->_M_impl._M_map_size,
> > > __nodes_to_add) + 2;
> > > + if (__new_map_size > ((max_size() + __deque_buf_size(sizeof(_Tp)) -
> > > 1)
> > > + / __deque_buf_size(sizeof(_Tp))) * 2)
> > > + __builtin_unreachable ();
> >
> > I think it's worth avoiding the duplicate call, for -O0 -std=c++98.
> > It makes the line shorter too:
> >
> > const size_t __bufsz = __deque_buf_size(sizeof(_Tp));
> > if (__new_map_size > ((max_size() + __bufsz - 1) / __bufsz) * 2)
> > __builtin_unreachable();
>
> This looks better, in fact that what I inteded with that forgotten
> veriable. I think __deque_buf_size is a constexpr and gets replaced by
> constant.
No in C++98 (there's no constexpr at all in C++98), and only if used
in a context that requires constant evaluation.
>
> I wonder why "const" is useful here.
Because if you don't initialize a constant expression, then you get a
runtime call to __deque_buf_size instead, which then has to be
inlined.
Using the call to initialize a const size_t ensures it's a constant
expression, and there's nothing for the inliner to do.
> Also I wonder if there is any
> difference between size_type and size_t, or it can be used
> interchangeably?
In general, they are not interchangeable. size_type might comes from
the allocator. But in libstdc++ all our containers (*except*
std::basic_string) just define size_type as a typedef for size_t.
Since __deque_buf_size returns a size_t, that seemed like the right
type (even though they happen to be the same in this code).
> For -O0 we do keep the cmp instruction around, but "optimize away" the
> conditonal jump to unreachable, which is bit odd. An option would be to
> add ifdef __OPTIMIZE__ guards.
>
> This is updated patch I am testing
OK for trunk if testing succeeds, thanks!
>
> diff --git a/libstdc++-v3/include/bits/deque.tcc
> b/libstdc++-v3/include/bits/deque.tcc
> index deb010a0ebb..ee03c917a29 100644
> --- a/libstdc++-v3/include/bits/deque.tcc
> +++ b/libstdc++-v3/include/bits/deque.tcc
> @@ -956,6 +956,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> + std::max(this->_M_impl._M_map_size,
> __nodes_to_add) + 2;
>
> + const size_t __bufsz = __deque_buf_size(sizeof(_Tp));
> + if (__new_map_size > ((max_size() + __bufsz - 1) / __bufsz) * 2)
> + __builtin_unreachable();
> +
> _Map_pointer __new_map = this->_M_allocate_map(__new_map_size);
> __new_nstart = __new_map + (__new_map_size - __new_num_nodes) / 2
> + (__add_at_front ? __nodes_to_add : 0);
> diff --git a/libstdc++-v3/include/bits/stl_deque.h
> b/libstdc++-v3/include/bits/stl_deque.h
> index c617933bd81..dd1212793de 100644
> --- a/libstdc++-v3/include/bits/stl_deque.h
> +++ b/libstdc++-v3/include/bits/stl_deque.h
> @@ -1266,7 +1266,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> _GLIBCXX_NODISCARD
> size_type
> size() const _GLIBCXX_NOEXCEPT
> - { return this->_M_impl._M_finish - this->_M_impl._M_start; }
> + {
> + size_type __sz = this->_M_impl._M_finish - this->_M_impl._M_start;
> + if (__sz > max_size ())
> + __builtin_unreachable ();
> + return __sz;
> + }
>
> /** Returns the size() of the largest possible %deque. */
> _GLIBCXX_NODISCARD
>
