From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42105 invoked by alias); 30 Apr 2015 08:41:06 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 42078 invoked by uid 89); 30 Apr 2015 08:41:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wg0-f47.google.com Received: from mail-wg0-f47.google.com (HELO mail-wg0-f47.google.com) (74.125.82.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 30 Apr 2015 08:41:04 +0000 Received: by wgyo15 with SMTP id o15so54230387wgy.2; Thu, 30 Apr 2015 01:41:01 -0700 (PDT) X-Received: by 10.180.99.166 with SMTP id er6mr3460337wib.58.1430383261051; Thu, 30 Apr 2015 01:41:01 -0700 (PDT) Received: from [192.168.0.22] (arf62-1-82-237-250-248.fbx.proxad.net. [82.237.250.248]) by mx.google.com with ESMTPSA id v20sm2178153wjr.49.2015.04.30.01.40.59 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 30 Apr 2015 01:41:00 -0700 (PDT) Message-ID: <5541EA9A.1010803@gmail.com> Date: Thu, 30 Apr 2015 09:00:00 -0000 From: =?windows-1252?Q?Fran=E7ois_Dumont?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Jonathan Wakely CC: "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: niter_base simplification References: <5538003D.30905@gmail.com> <20150427115515.GG3618@redhat.com> In-Reply-To: <20150427115515.GG3618@redhat.com> Content-Type: multipart/mixed; boundary="------------050400010303080503020107" X-SW-Source: 2015-04/txt/msg01981.txt.bz2 This is a multi-part message in MIME format. --------------050400010303080503020107 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Content-length: 4709 On 27/04/2015 13:55, Jonathan Wakely wrote: > On 22/04/15 22:10 +0200, François Dumont wrote: >> Hello >> >> I don't know if I am missing something but I think __niter_base >> could be simplified to remove usage of _Iter_base. Additionally I >> overload it to also remove __normal_iterator layer even if behind a >> reverse_iterator or move_iterator, might help compiler to optimize >> code, no ? If not, might allow other algo optimization in the future... >> >> I prefered to provide a __make_reverse_iterator to allow the >> latter in C++11 and not only in C++14. Is it fine to do it this way >> or do you prefer to simply get rid of all this part ? > > It's fine to add __make_reverse_iterator but see my comment below. > >> * include/bits/cpp_type_traits.h (__gnu_cxx::__normal_iterator): >> Delete. > > You're removing __is_normal_iterator not __normal_iterator. > >> * include/bits/stl_algobase.h (std::__niter_base): Adapt. >> * include/bits/stl_iterator.h (__make_reverse_iterator): New in >> C++11. >> (std::__niter_base): Overloads for std::reverse_iterator, >> __gnu_cxx::__normal_iterator and std::move_iterator. >> >> Tested under Linux x86_64. I checked that std::copy still ends up >> calling __builtin_memmove when used on vector iterators. >> >> François >> > >> diff --git a/libstdc++-v3/include/bits/stl_algobase.h >> b/libstdc++-v3/include/bits/stl_algobase.h >> index 0bcb133..73eea6b 100644 >> --- a/libstdc++-v3/include/bits/stl_algobase.h >> +++ b/libstdc++-v3/include/bits/stl_algobase.h >> @@ -270,17 +270,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> return __a; >> } >> >> - // If _Iterator is a __normal_iterator return its base (a plain >> pointer, >> - // normally) otherwise return it untouched. See copy, fill, ... >> + // Fallback implementation of the function used to remove the >> + // __normal_iterator wrapper. See copy, fill, ... > > It's a bit strange to have a function with no other overloads visible > described as a fallback. It would be good to say that the other > definition is in bits/stl_iterator.h > >> template >> - struct _Niter_base >> - : _Iter_base<_Iterator, __is_normal_iterator<_Iterator>::__value> >> - { }; >> - >> - template >> - inline typename _Niter_base<_Iterator>::iterator_type >> + inline _Iterator >> __niter_base(_Iterator __it) >> - { return std::_Niter_base<_Iterator>::_S_base(__it); } >> + { return __it; } >> >> // Likewise, for move_iterator. > > This comment no longer makes sense, because you've removed the comment > on _Niter_base that it referred to. Please restore the original text > of the _Niter_base comment for _Miter_base. > > (Alternatively, could the same simplification be made for > __miter_base? Do we need _Miter_base<> or just two overloads of > __miter_base()?) Definitely, I already have a patch for that. > > >> template >> diff --git a/libstdc++-v3/include/bits/stl_iterator.h >> b/libstdc++-v3/include/bits/stl_iterator.h >> index 4a9189e..3aad9f3 100644 >> --- a/libstdc++-v3/include/bits/stl_iterator.h >> +++ b/libstdc++-v3/include/bits/stl_iterator.h >> @@ -390,7 +390,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> { return __y.base() - __x.base(); } >> //@} >> >> -#if __cplusplus > 201103L >> +#if __cplusplus == 201103L >> + template >> + inline reverse_iterator<_Iterator> >> + __make_reverse_iterator(_Iterator __i) >> + { return reverse_iterator<_Iterator>(__i); } >> + >> +# define _GLIBCXX_MAKE_REVERSE_ITERATOR(_Iter) \ >> + std::__make_reverse_iterator(_Iter) >> +#elif __cplusplus > 201103L >> #define __cpp_lib_make_reverse_iterator 201402 >> >> // _GLIBCXX_RESOLVE_LIB_DEFECTS >> @@ -400,6 +408,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> inline reverse_iterator<_Iterator> >> make_reverse_iterator(_Iterator __i) >> { return reverse_iterator<_Iterator>(__i); } >> + >> +# define _GLIBCXX_MAKE_REVERSE_ITERATOR(_Iter) \ >> + std::make_reverse_iterator(_Iter) >> +#endif >> + >> +#if __cplusplus >= 201103L >> + template >> + auto >> + __niter_base(reverse_iterator<_Iterator> __it) >> + -> >> decltype(_GLIBCXX_MAKE_REVERSE_ITERATOR(__niter_base(__it.base()))) >> + { return >> _GLIBCXX_MAKE_REVERSE_ITERATOR(__niter_base(__it.base())); } >> #endif >> > > It might be simpler to just add __make_reverse_iterator for >= 201103L > and then always use std::__make_reverse_iterator instead of a macro. > > That's similar to what we do for std:__addressof and std:addressof. > Ok, attached is the patch I have plan to commit then that I am testing at the moment. François --------------050400010303080503020107 Content-Type: text/x-patch; name="niter.patch" Content-Transfer-Encoding: 8bit Content-Disposition: attachment; filename="niter.patch" Content-length: 5235 Index: ChangeLog =================================================================== --- ChangeLog (revision 222611) +++ ChangeLog (working copy) @@ -1,5 +1,14 @@ 2015-04-30 François Dumont + * include/bits/cpp_type_traits.h + (__gnu_cxx::__is_normal_iterator): Delete. + * include/bits/stl_algobase.h (std::__niter_base): Adapt. + * include/bits/stl_iterator.h (__make_reverse_iterator): New in C++11. + (std::__niter_base): Overloads for std::reverse_iterator, + __gnu_cxx::__normal_iterator and std::move_iterator. + +2015-04-30 François Dumont + * include/bits/hashtable_policy.h (_Prime_rehash_policy::_S_n_primes): Delete. * src/c++11/hashtable_c++0x.cc (_Prime_rehash_policy::_M_next_bkt): Index: include/bits/cpp_type_traits.h =================================================================== --- include/bits/cpp_type_traits.h (revision 222610) +++ include/bits/cpp_type_traits.h (working copy) @@ -64,17 +64,6 @@ // removed. // -// Forward declaration hack, should really include this from somewhere. -namespace __gnu_cxx _GLIBCXX_VISIBILITY(default) -{ -_GLIBCXX_BEGIN_NAMESPACE_VERSION - - template - class __normal_iterator; - -_GLIBCXX_END_NAMESPACE_VERSION -} // namespace - namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION @@ -331,24 +320,6 @@ }; // - // Normal iterator type - // - template - struct __is_normal_iterator - { - enum { __value = 0 }; - typedef __false_type __type; - }; - - template - struct __is_normal_iterator< __gnu_cxx::__normal_iterator<_Iterator, - _Container> > - { - enum { __value = 1 }; - typedef __true_type __type; - }; - - // // An arithmetic type is an integer type or a floating point type // template Index: include/bits/stl_algobase.h =================================================================== --- include/bits/stl_algobase.h (revision 222610) +++ include/bits/stl_algobase.h (working copy) @@ -270,19 +270,15 @@ return __a; } - // If _Iterator is a __normal_iterator return its base (a plain pointer, - // normally) otherwise return it untouched. See copy, fill, ... + // Fallback implementation of the function in bits/stl_iterator.h used to + // remove the __normal_iterator wrapper. See copy, fill, ... template - struct _Niter_base - : _Iter_base<_Iterator, __is_normal_iterator<_Iterator>::__value> - { }; - - template - inline typename _Niter_base<_Iterator>::iterator_type + inline _Iterator __niter_base(_Iterator __it) - { return std::_Niter_base<_Iterator>::_S_base(__it); } + { return __it; } - // Likewise, for move_iterator. + // If _Iterator is a move_iterator return its base otherwise return it + // untouched. See copy, fill, ... template struct _Miter_base : _Iter_base<_Iterator, __is_move_iterator<_Iterator>::__value> Index: include/bits/stl_iterator.h =================================================================== --- include/bits/stl_iterator.h (revision 222610) +++ include/bits/stl_iterator.h (working copy) @@ -388,9 +388,16 @@ { return __y.base() - __x.base(); } //@} -#if __cplusplus > 201103L -#define __cpp_lib_make_reverse_iterator 201402 +#if __cplusplus >= 201103L + // Same as C++14 make_reverse_iterator but used in C++03 mode too. + template + inline reverse_iterator<_Iterator> + __make_reverse_iterator(_Iterator __i) + { return reverse_iterator<_Iterator>(__i); } +# if __cplusplus > 201103L +# define __cpp_lib_make_reverse_iterator 201402 + // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 2285. make_reverse_iterator /// Generator function for reverse_iterator. @@ -398,8 +405,17 @@ inline reverse_iterator<_Iterator> make_reverse_iterator(_Iterator __i) { return reverse_iterator<_Iterator>(__i); } +# endif #endif +#if __cplusplus >= 201103L + template + auto + __niter_base(reverse_iterator<_Iterator> __it) + -> decltype(__make_reverse_iterator(__niter_base(__it.base()))) + { return __make_reverse_iterator(__niter_base(__it.base())); } +#endif + // 24.4.2.2.1 back_insert_iterator /** * @brief Turns assignment into insertion. @@ -935,6 +951,18 @@ _GLIBCXX_END_NAMESPACE_VERSION } // namespace +namespace std _GLIBCXX_VISIBILITY(default) +{ +_GLIBCXX_BEGIN_NAMESPACE_VERSION + + template + _Iterator + __niter_base(__gnu_cxx::__normal_iterator<_Iterator, _Container> __it) + { return __it.base(); } + +_GLIBCXX_END_NAMESPACE_VERSION +} // namespace + #if __cplusplus >= 201103L namespace std _GLIBCXX_VISIBILITY(default) @@ -1168,6 +1196,12 @@ // @} group iterators + template + auto + __niter_base(move_iterator<_Iterator> __it) + -> decltype(make_move_iterator(__niter_base(__it.base()))) + { return make_move_iterator(__niter_base(__it.base())); } + _GLIBCXX_END_NAMESPACE_VERSION } // namespace --------------050400010303080503020107--