Re: [Patch] libstdc++/23425

[chris jefferson <caj at cs dot york dot ac dot uk>]

Paolo Carlini wrote:

>Howard Hinnant wrote:
>
>  
>
>>On the issue of inlining, and I'm speaking from an standpoint of 
>>great ignorance here, it could well be that __erase_from is a 
>>function that we want inlined if it is optimized (i.e. when the 
>>destructor is trivial), and not inlined if it is not optimized.  We 
>>can overload __erase_from on tr1::has_trivial_destructor<T>::value, 
>>inlining for true and not for false.
>>    
>>
>
>By the way, I think I have just discovered something funny in
>stl_construct.h, related to that issue.
>
>In stl_construct.h we have different versions of _Destroy, depending on
>the allocator (std::allocator or custom) and the type (trivial
>destructor or not, indeed). The worst case it's something like the below
>and since we don't have accurate type_traits we have to fall back to it
>for most types, besides scalars:
>
>  template<typename _ForwardIterator, typename _Allocator>
>    void
>    _Destroy(_ForwardIterator __first, _ForwardIterator __last,
>             _Allocator __alloc)
>    {
>      for (; __first != __last; ++__first)
>        __alloc.destroy(&*__first);
>    }
>
>  
>
For vectors, just replacing "first != last" with "first < last" is
enough to let the loop optimisers remove the loop. This is PR 23361.
It's annoying that the != gets left behind and the < gets removed, as
clearly != is the most generic looping method from the point of view of
iterators.

Unfortunatly, the optimiser seems to (not unreasonably) always get
confused by deque iterators, and the loop optimisers never seem to kick
in at all.

>Now, the funny thing is that this template is used only for std::deque
>and std::vector, which both support random access iterators! If I
>rewrite the loop exploiting that property, something like:
>
>  template<typename _RandomIterator, typename _Allocator>
>    void
>    _Destroy(_RandomIterator __first, _RandomIterator __last,
>             _Allocator __alloc)
>    {
>      typename iterator_traits<_RandomIterator>::difference_type __diff =
>        __last - __first;
>
>      for (; __diff > 0; --__diff, ++__first)
>        __alloc.destroy(&*__first);
>    }
>
>our current loop optimizer is able to optimize to nothing the loop for
>most POD types, at least!
>
>To good and easy to be true?
>
>  
>
In the case where it isn't removed, I find that this loop is slightly
faster for deques, and slightly slower for vectors. Depends on what
tradeoff should be made really.

Chris