[PATCH] libstdc++: Add unreachable checks to begin/end [PR111499]

[Andrew Pinski]

On Fri, Apr 4, 2025 at 3:37 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> To help along cases where begin and end are passed on to another function
> and then uses std::distance on them. In the case of the testcases added,
> std::lexicographical_compare_three_way (for C++20). We now able to opimize
> `vector < known empty vector` to a constant where before we would need to load
> begin() and end() and do a comparison of them.
>
> Note for C++20+ at -O1 we still don't optimize it to a constant but that is
> due to not doing copy propagation for aggregates.
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
>         PR libstdc++/111499
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/stl_vector.h (vector::begin(), vector::end(),
>         vector::cbegin(), vector::cend()):  Add __builtin_unreachable call to
>         announce that _M_finish is greater than or equal to _M_start.

Ignore this version of the patch, I submitted the wrong version of it.
the calls were supposed to be under `#ifdef __OPTIMIZE__` as suggested
in the bug report and that is what I tested too but I forgot to `git
commit --amend -a` on my git tree before submitting it.

Sorry about that,
Andrew

>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/tree-ssa/pr111499-1.C: New test.
>         * g++.dg/tree-ssa/pr111499.C: New test.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/testsuite/g++.dg/tree-ssa/pr111499-1.C | 16 ++++++++++
>  gcc/testsuite/g++.dg/tree-ssa/pr111499.C   | 15 +++++++++
>  libstdc++-v3/include/bits/stl_vector.h     | 36 ++++++++++++++++++----
>  3 files changed, 61 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr111499-1.C
>  create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr111499.C
>
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr111499-1.C b/gcc/testsuite/g++.dg/tree-ssa/pr111499-1.C
> new file mode 100644
> index 00000000000..737f6673301
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/pr111499-1.C
> @@ -0,0 +1,16 @@
> +// { dg-do compile }
> +// { dg-options "-O1 -fdump-tree-optimized -Wextra -Wall" }
> +#include <vector>
> +
> +bool f(const std::vector<unsigned char> &v)
> +{
> +  std::vector<unsigned char> w;
> +  return v < w;
> +}
> +// This should compile to empty function; check that no size of
> +// vector is determined and there is no allocation
> +// For C++20+, at -O1 there is a missed optimization dealing with copy propagation for aggregates
> +// { dg-final { scan-tree-dump-not "_M_start" "optimized" { xfail c++20 } } }
> +// { dg-final { scan-tree-dump-not "delete" "optimized" } }
> +// { dg-final { scan-tree-dump-not " = MEM" "optimized" } }
> +// { dg-final { scan-tree-dump "return 0" "optimized" { xfail c++20 } } }
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr111499.C b/gcc/testsuite/g++.dg/tree-ssa/pr111499.C
> new file mode 100644
> index 00000000000..faa38299d4d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/pr111499.C
> @@ -0,0 +1,15 @@
> +// { dg-do compile }
> +// { dg-options "-O2 -fdump-tree-optimized -Wextra -Wall" }
> +#include <vector>
> +
> +bool f(const std::vector<unsigned char> &v)
> +{
> +  std::vector<unsigned char> w;
> +  return v < w;
> +}
> +// This should compile to empty function; check that no size of
> +// vector is determined and there is no allocation
> +// { dg-final { scan-tree-dump-not "_M_start" "optimized" } }
> +// { dg-final { scan-tree-dump-not "delete" "optimized" } }
> +// { dg-final { scan-tree-dump-not " = MEM" "optimized" } }
> +// { dg-final { scan-tree-dump "return 0" "optimized" } }
> diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h
> index 458adc987da..aa72a5de709 100644
> --- a/libstdc++-v3/include/bits/stl_vector.h
> +++ b/libstdc++-v3/include/bits/stl_vector.h
> @@ -996,7 +996,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>        _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>        iterator
>        begin() _GLIBCXX_NOEXCEPT
> -      { return iterator(this->_M_impl._M_start); }
> +      {
> +       if (this->_M_impl._M_start > this->_M_impl._M_finish)
> +         __builtin_unreachable();
> +       return iterator(this->_M_impl._M_start);
> +      }
>
>        /**
>         *  Returns a read-only (constant) iterator that points to the
> @@ -1006,7 +1010,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>        _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>        const_iterator
>        begin() const _GLIBCXX_NOEXCEPT
> -      { return const_iterator(this->_M_impl._M_start); }
> +      {
> +       if (this->_M_impl._M_start > this->_M_impl._M_finish)
> +         __builtin_unreachable();
> +       return const_iterator(this->_M_impl._M_start);
> +      }
>
>        /**
>         *  Returns a read/write iterator that points one past the last
> @@ -1016,7 +1024,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>        _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>        iterator
>        end() _GLIBCXX_NOEXCEPT
> -      { return iterator(this->_M_impl._M_finish); }
> +      {
> +       if (this->_M_impl._M_start > this->_M_impl._M_finish)
> +         __builtin_unreachable();
> +       return iterator(this->_M_impl._M_finish);
> +      }
>
>        /**
>         *  Returns a read-only (constant) iterator that points one past
> @@ -1026,7 +1038,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>        _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>        const_iterator
>        end() const _GLIBCXX_NOEXCEPT
> -      { return const_iterator(this->_M_impl._M_finish); }
> +      {
> +       if (this->_M_impl._M_start > this->_M_impl._M_finish)
> +         __builtin_unreachable();
> +       return const_iterator(this->_M_impl._M_finish);
> +      }
>
>        /**
>         *  Returns a read/write reverse iterator that points to the
> @@ -1077,7 +1093,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>        [[__nodiscard__]] _GLIBCXX20_CONSTEXPR
>        const_iterator
>        cbegin() const noexcept
> -      { return const_iterator(this->_M_impl._M_start); }
> +      {
> +       if (this->_M_impl._M_start > this->_M_impl._M_finish)
> +         __builtin_unreachable();
> +       return const_iterator(this->_M_impl._M_start);
> +      }
>
>        /**
>         *  Returns a read-only (constant) iterator that points one past
> @@ -1087,7 +1107,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>        [[__nodiscard__]] _GLIBCXX20_CONSTEXPR
>        const_iterator
>        cend() const noexcept
> -      { return const_iterator(this->_M_impl._M_finish); }
> +      {
> +       if (this->_M_impl._M_start > this->_M_impl._M_finish)
> +         __builtin_unreachable();
> +       return const_iterator(this->_M_impl._M_finish);
> +      }
>
>        /**
>         *  Returns a read-only (constant) reverse iterator that points
> --
> 2.43.0
>