This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [v3 PATCH] Don't revisit a variant we are already visiting.


On 28/03/19 15:21 +0200, Ville Voutilainen wrote:
On Thu, 28 Mar 2019 at 15:07, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:

This shaves off some unnecessary codegen. In the assignment
operators and swap, we are already visiting the rhs, so we shouldn't
visit it again to get to its guts, we already have them in-hand.

2019-03-28  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Don't revisit a variant we are already visiting.
    * include/std/variant (__variant_construct_single): New.
    (__variant_construct): Use it.
    (_M_destructive_move): Likewise, turn into a template.
    (_M_destructive_copy): Likewise.
    (_Copy_assign_base::operator=): Adjust.
    (_Move_assign_base::operator=): Likewise.
    (swap): Likewise.

Minor adjustment, use direct-init in all cases:

-+                  auto __tmp = std::move(__rhs_mem);
++                  auto __tmp(std::move(__rhs_mem));

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 3932a8a..fe96cc8 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -428,20 +428,25 @@ namespace __variant
    using _Variant_storage_alias =
	_Variant_storage<_Traits<_Types...>::_S_trivial_dtor, _Types...>;

+  template<typename _Tp, typename _Up>
+  void __variant_construct_single(_Tp&& __lhs, _Up&& __rhs_mem)

Please indent this line to match the opening brace, not the
template-head.

@@ -490,34 +495,38 @@ namespace __variant
	  std::forward<_Move_ctor_base>(__rhs));
      }

-      void _M_destructive_move(_Move_ctor_base&& __rhs)
-      {
-	this->_M_reset();
-	__try
-	  {
-	    __variant_construct<_Types...>(*this,
-	      std::forward<_Move_ctor_base>(__rhs));
-	  }
-	__catch (...)
-	  {
-	    this->_M_index = variant_npos;
-	    __throw_exception_again;
-	  }
-      }
+      template<typename _Idx, typename _Up>
+        void _M_destructive_move(_Idx __rhs_index, _Up&& __rhs)

Does __rhs_index need to be a template parameter in these function
templates? It seems to always be passed size_t, but the value will
always fit in unsigned short. Could it just use size_t? Or even
unsigned, since we don't really need to pass 64 bits?

I think typename _Base::__index_type is strictly correct, but that's a
lot to type and to read.

OK with the indentation tweak, and I'll leave changing the _Idx or not
to your judgment.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]