On 18/06/18 23:01 +0200, François Dumont wrote:
Hi
I abandon the idea of providing Debug algos, it would be too much
code to add and maintain. However I haven't quit on reducing Debug
mode performance impact.
So this patch make use of the existing std::__niter_base to get
rid of the debug layer around __gnu_debug::vector<>::iterator so that
__builtin_memmove replacement can take place.
As _Safe_iterator<> do not have a constructor taking a pointer I
change algos implementation so that we do not try to instantiate the
iterator type ourselves but rather rely on its operators + or -.
The small drawback is that for std::equal algo where we can't use
the __glibcxx_can_increment we need to keep the debug layer to make
sure we don't reach past-the-end iterator. So I had to remove usage of
__niter_base when in Debug mode, doing so it also disable potential
usage of __builtin_memcmp when calling std::equal on
std::__cxx1998::vector<> iterators. A rather rare situation I think.
Note that I don't know how to test that __builtin_memmove has been
indeed used. So I've been through some debug sessions to check that.
The attached patch (not fully tested) seems to be a much simpler way
to achieve the same thing. Instead of modifying all the helper
structs, just define a new function to re-wrap the result into the
desired iterator type.
diff --git a/libstdc++-v3/include/debug/stl_iterator.h
b/libstdc++-v3/include/debug/stl_iterator.h
index a6a2a76..eca7203 100644
--- a/libstdc++-v3/include/debug/stl_iterator.h
+++ b/libstdc++-v3/include/debug/stl_iterator.h
@@ -120,4 +120,19 @@ namespace __gnu_debug
#endif
}
+#if __cplusplus >= 201103L
+namespace std
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+template<typename _Iterator, typename _Container, typename _Sequence>
+ _Iterator
+ __niter_base(const __gnu_debug::_Safe_iterator<
+ __gnu_cxx::__normal_iterator<_Iterator, _Container>,
+ _Sequence>&);
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+#endif
Why is this overload only defined for C++11 and later? I defined it
unconditionally in the attached patch.
What do you think?
diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 022a3f1598b..91d673e6c81 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -415,13 +415,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__copy_move_a2(istreambuf_iterator<_CharT, char_traits<_CharT> >,
istreambuf_iterator<_CharT, char_traits<_CharT> >, _CharT*);
+#ifdef _GLIBCXX_DEBUG
+ template<typename _Base, typename _Seq, typename _From>
+ inline __gnu_debug::_Safe_iterator<_Base, _Seq>
+ __wrap_iter(__gnu_debug::_Safe_iterator<_Base, _Seq> __safe, _From __from)
+ {
+ return __gnu_debug::_Safe_iterator<_Base, _Seq>(_Base(__from),
+ __safe._M_sequence);
+ }
+#endif
+
+ template<typename _To, typename _From>
+ inline _To
+ __wrap_iter(_To, _From __from)
+ { return _To(__from); }
+
template<bool _IsMove, typename _II, typename _OI>
inline _OI
__copy_move_a2(_II __first, _II __last, _OI __result)
{
- return _OI(std::__copy_move_a<_IsMove>(std::__niter_base(__first),
- std::__niter_base(__last),
- std::__niter_base(__result)));
+ return std::__wrap_iter(__result,
+ std::__copy_move_a<_IsMove>(std::__niter_base(__first),
+ std::__niter_base(__last),
+ std::__niter_base(__result)));
}
/**
@@ -593,9 +609,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
inline _BI2
__copy_move_backward_a2(_BI1 __first, _BI1 __last, _BI2 __result)
{
- return _BI2(std::__copy_move_backward_a<_IsMove>
- (std::__niter_base(__first), std::__niter_base(__last),
- std::__niter_base(__result)));
+ return std::__wrap_iter(__result,
+ std::__copy_move_backward_a<_IsMove>(std::__niter_base(__first),
+ std::__niter_base(__last),
+ std::__niter_base(__result)));
}
/**
@@ -785,7 +802,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__glibcxx_function_requires(_OutputIteratorConcept<_OI, _Tp>)
__glibcxx_requires_can_increment(__first, __n);
- return _OI(std::__fill_n_a(std::__niter_base(__first), __n, __value));
+ return std::__wrap_iter(__first,
+ std::__fill_n_a(std::__niter_base(__first), __n, __value));
}
template<bool _BoolType>
diff --git a/libstdc++-v3/include/debug/stl_iterator.h b/libstdc++-v3/include/debug/stl_iterator.h
index a6a2a762766..431293bdec9 100644
--- a/libstdc++-v3/include/debug/stl_iterator.h
+++ b/libstdc++-v3/include/debug/stl_iterator.h
@@ -118,6 +118,19 @@ namespace __gnu_debug
-> decltype(std::make_move_iterator(__base(__it.base())))
{ return std::make_move_iterator(__base(__it.base())); }
#endif
-}
+} // namespace __gnu_debug
+
+namespace std
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+template<typename _Iterator, typename _Container, typename _Sequence>
+ _Iterator
+ __niter_base(const __gnu_debug::_Safe_iterator<
+ __gnu_cxx::__normal_iterator<_Iterator, _Container>,
+ _Sequence>&);
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace std
#endif
diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector
index 8d60da328e1..54697030974 100644
--- a/libstdc++-v3/include/debug/vector
+++ b/libstdc++-v3/include/debug/vector
@@ -784,7 +784,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
};
_GLIBCXX_END_NAMESPACE_VERSION
-#endif
+#endif // C++11
+
+ template<typename _Iterator, typename _Container, typename _Sequence>
+ _Iterator
+ __niter_base(const __gnu_debug::_Safe_iterator<
+ __gnu_cxx::__normal_iterator<_Iterator, _Container>,
+ _Sequence>& __it)
+ { return std::__niter_base(__it.base()); }
} // namespace std
