libstdc++: Allow adjacent __maybe_present_t<false, ...> to overlap

Message ID 20240401221539.842841-1-ppalka@redhat.com
State New
Headers
Series libstdc++: Allow adjacent __maybe_present_t<false, ...> to overlap |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Patrick Palka April 1, 2024, 10:15 p.m. UTC
  Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

-- >8 --

Currently __maybe_present_t<false, T> maps to the same empty class
type independent of T.  This is suboptimal because it means adjacent
__maybe_present_t<false, ...> members with the [[no_unique_address]]
attribute can't overlap even if the conditionally present types are
different.

This patch fixes this by turning this empty class type into a template
parameterized by the conditionally present type, so that

  [[no_unique_address]] __maybe_present_t<false, T> _M_a;
  [[no_unique_address]] __maybe_present_t<false, U> _M_b;

now overlap if T and U are different.

This patch goes a step further and also adds an optional integer
discriminator parameter to allow for overlapping when T and U are
the same.

libstdc++-v3/ChangeLog:

	* include/std/ranges (ranges::__detail::_Empty): Rename to ...
	(ranges::__detail::_Absent): ... this.  Turn into a template
	parameterized by the absent type _Tp and discriminator _Disc.
	(ranges::__detail::__maybe_present_t): Add an optional
	discriminator parameter.
	(slide_view::_M_cached_begin): Pass a discriminator argument to
	__maybe_present_t.
	(slide_view::_M_cached_end): Likewise.
	* testsuite/std/ranges/adaptors/sizeof.cc: Verify the size of
	slide_view<V> is 3 instead 4 pointers.
---
 libstdc++-v3/include/std/ranges                     | 13 ++++++++-----
 .../testsuite/std/ranges/adaptors/sizeof.cc         |  4 ++++
 2 files changed, 12 insertions(+), 5 deletions(-)
  

Comments

Jonathan Wakely April 2, 2024, 9:17 a.m. UTC | #1
On Mon, 1 Apr 2024 at 23:16, Patrick Palka <ppalka@redhat.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

This is a layout change for some specializations of slide_view, but
better to do that now than change it between gcc 14 and 15.

OK for trunk.


>
> -- >8 --
>
> Currently __maybe_present_t<false, T> maps to the same empty class
> type independent of T.  This is suboptimal because it means adjacent
> __maybe_present_t<false, ...> members with the [[no_unique_address]]
> attribute can't overlap even if the conditionally present types are
> different.
>
> This patch fixes this by turning this empty class type into a template
> parameterized by the conditionally present type, so that
>
>   [[no_unique_address]] __maybe_present_t<false, T> _M_a;
>   [[no_unique_address]] __maybe_present_t<false, U> _M_b;
>
> now overlap if T and U are different.
>
> This patch goes a step further and also adds an optional integer
> discriminator parameter to allow for overlapping when T and U are
> the same.
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/ranges (ranges::__detail::_Empty): Rename to ...
>         (ranges::__detail::_Absent): ... this.  Turn into a template
>         parameterized by the absent type _Tp and discriminator _Disc.
>         (ranges::__detail::__maybe_present_t): Add an optional
>         discriminator parameter.
>         (slide_view::_M_cached_begin): Pass a discriminator argument to
>         __maybe_present_t.
>         (slide_view::_M_cached_end): Likewise.
>         * testsuite/std/ranges/adaptors/sizeof.cc: Verify the size of
>         slide_view<V> is 3 instead 4 pointers.
> ---
>  libstdc++-v3/include/std/ranges                     | 13 ++++++++-----
>  .../testsuite/std/ranges/adaptors/sizeof.cc         |  4 ++++
>  2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 7d739852677..afce818376b 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -886,14 +886,17 @@ namespace views
>
>  namespace __detail
>  {
> -  struct _Empty { };
> +  template<typename _Tp, int _Disc>
> +    struct _Absent { };
>
>    // Alias for a type that is conditionally present
>    // (and is an empty type otherwise).
>    // Data members using this alias should use [[no_unique_address]] so that
>    // they take no space when not needed.
> -  template<bool _Present, typename _Tp>
> -    using __maybe_present_t = __conditional_t<_Present, _Tp, _Empty>;
> +  // The optional template parameter _Disc is for discriminating two otherwise
> +  // equivalent absent types so that even they can overlap.
> +  template<bool _Present, typename _Tp, int _Disc = 0>
> +    using __maybe_present_t = __conditional_t<_Present, _Tp, _Absent<_Tp, _Disc>>;
>
>    // Alias for a type that is conditionally const.
>    template<bool _Const, typename _Tp>
> @@ -6553,10 +6556,10 @@ namespace views::__adaptor
>      range_difference_t<_Vp> _M_n;
>      [[no_unique_address]]
>        __detail::__maybe_present_t<__detail::__slide_caches_first<_Vp>,
> -                                 __detail::_CachedPosition<_Vp>> _M_cached_begin;
> +                                 __detail::_CachedPosition<_Vp>, 0> _M_cached_begin;
>      [[no_unique_address]]
>        __detail::__maybe_present_t<__detail::__slide_caches_last<_Vp>,
> -                                 __detail::_CachedPosition<_Vp>> _M_cached_end;
> +                                 __detail::_CachedPosition<_Vp>, 1> _M_cached_end;
>
>      template<bool> class _Iterator;
>      class _Sentinel;
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
> index 12a9da3181d..08c01704d10 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
> @@ -49,3 +49,7 @@ static_assert(sizeof(ranges::lazy_split_view<V, std::string_view>) == 4*ptr);
>
>  static_assert
>   (sizeof(ranges::reverse_view<ranges::filter_view<V, decltype(pred_l)>>) == 3*ptr);
> +
> +#if __cpp_lib_ranges_slide
> +static_assert(sizeof(ranges::slide_view<V>) == 3*ptr);
> +#endif
> --
> 2.44.0.448.gc2cbfbd2e2
>
  

Patch

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 7d739852677..afce818376b 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -886,14 +886,17 @@  namespace views
 
 namespace __detail
 {
-  struct _Empty { };
+  template<typename _Tp, int _Disc>
+    struct _Absent { };
 
   // Alias for a type that is conditionally present
   // (and is an empty type otherwise).
   // Data members using this alias should use [[no_unique_address]] so that
   // they take no space when not needed.
-  template<bool _Present, typename _Tp>
-    using __maybe_present_t = __conditional_t<_Present, _Tp, _Empty>;
+  // The optional template parameter _Disc is for discriminating two otherwise
+  // equivalent absent types so that even they can overlap.
+  template<bool _Present, typename _Tp, int _Disc = 0>
+    using __maybe_present_t = __conditional_t<_Present, _Tp, _Absent<_Tp, _Disc>>;
 
   // Alias for a type that is conditionally const.
   template<bool _Const, typename _Tp>
@@ -6553,10 +6556,10 @@  namespace views::__adaptor
     range_difference_t<_Vp> _M_n;
     [[no_unique_address]]
       __detail::__maybe_present_t<__detail::__slide_caches_first<_Vp>,
-				  __detail::_CachedPosition<_Vp>> _M_cached_begin;
+				  __detail::_CachedPosition<_Vp>, 0> _M_cached_begin;
     [[no_unique_address]]
       __detail::__maybe_present_t<__detail::__slide_caches_last<_Vp>,
-				  __detail::_CachedPosition<_Vp>> _M_cached_end;
+				  __detail::_CachedPosition<_Vp>, 1> _M_cached_end;
 
     template<bool> class _Iterator;
     class _Sentinel;
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
index 12a9da3181d..08c01704d10 100644
--- a/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
@@ -49,3 +49,7 @@  static_assert(sizeof(ranges::lazy_split_view<V, std::string_view>) == 4*ptr);
 
 static_assert
  (sizeof(ranges::reverse_view<ranges::filter_view<V, decltype(pred_l)>>) == 3*ptr);
+
+#if __cpp_lib_ranges_slide
+static_assert(sizeof(ranges::slide_view<V>) == 3*ptr);
+#endif