Re: [PATCH v1] libstdc++: Optimize removal from unique assoc containers [PR112934]
Hi
2024. március 13., szerda 12:43 keltezéssel, Jonathan Wakely
írta:
> On Mon, 11 Mar 2024 at 23:36, Barnabás Pőcze wrote:
> >
> > Previously, calling erase(key) on both std::map and std::set
> > would execute that same code that std::multi{map,set} would.
> > However, doing that is unnecessary because std::{map,set}
> > guarantee that all elements are unique.
> >
> > It is reasonable to expect that erase(key) is equivalent
> > or better than:
> >
> > auto it = m.find(key);
> > if (it != m.end())
> > m.erase(it);
> >
> > However, this was not the case. Fix that by adding a new
> > function _Rb_tree<>::_M_erase_unique() that is essentially
> > equivalent to the above snippet, and use this from both
> > std::map and std::set.
>
> Hi, this change looks reasonable, thanks for the patch. Please note
> that GCC is currently in "stage 3" of its dev process so this change
> would have to wait until after GCC 14 branches from trunk, due in a
> few weeks.
OK; I didn't know that, thanks for telling me.
>
> I assume you ran the testsuite with no regressions. [...]
I hope so. I ran `make check-target-libstdc++-v3`, and it did not note any
unexpected failures as far as I can see:
Native configuration is x86_64-pc-linux-gnu
=== libstdc++ tests ===
Schedule of variations:
unix
Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for
target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /gcc/libstdc++-v3/testsuite/config/default.exp as
tool-and-target-specific interface file.
Running /gcc/libstdc++-v3/testsuite/libstdc++-abi/abi.exp ...
Running /gcc/libstdc++-v3/testsuite/libstdc++-dg/conformance.exp ...
Running
/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/prettyprinters.exp ...
Running /gcc/libstdc++-v3/testsuite/libstdc++-xmethods/xmethods.exp ...
=== libstdc++ Summary ===
# of expected passes 18646
# of expected failures126
# of unsupported tests672
> [...] Do you have benchmarks to show this making a difference?
As for benchmarks, I do not have any. But even if the performance does not
improve appreciably, the size of the generated code will definitely be smaller.
And in the end, the excessive code was the reason I opened the mentioned
issue[0]
in the first place, which should be eliminated hopefully.
> [...]
Regards,
Barnabás Pőcze
[0]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112934
[PATCH v1] libstdc++: Optimize removal from unique assoc containers [PR112934]
Previously, calling erase(key) on both std::map and std::set
would execute that same code that std::multi{map,set} would.
However, doing that is unnecessary because std::{map,set}
guarantee that all elements are unique.
It is reasonable to expect that erase(key) is equivalent
or better than:
auto it = m.find(key);
if (it != m.end())
m.erase(it);
However, this was not the case. Fix that by adding a new
function _Rb_tree<>::_M_erase_unique() that is essentially
equivalent to the above snippet, and use this from both
std::map and std::set.
libstdc++-v3/ChangeLog:
PR libstdc++/112934
* include/bits/stl_tree.h (_Rb_tree<>::_M_erase_unique): Add.
* include/bits/stl_map.h (map<>::erase): Use _M_erase_unique.
* include/bits/stl_set.h (set<>::erase): Likewise.
---
libstdc++-v3/include/bits/stl_map.h | 2 +-
libstdc++-v3/include/bits/stl_set.h | 2 +-
libstdc++-v3/include/bits/stl_tree.h | 17 +
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/libstdc++-v3/include/bits/stl_map.h
b/libstdc++-v3/include/bits/stl_map.h
index ad58a631af5..229643b77fd 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -1115,7 +1115,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
size_type
erase(const key_type& __x)
- { return _M_t.erase(__x); }
+ { return _M_t._M_erase_unique(__x); }
#if __cplusplus >= 201103L
// _GLIBCXX_RESOLVE_LIB_DEFECTS
diff --git a/libstdc++-v3/include/bits/stl_set.h
b/libstdc++-v3/include/bits/stl_set.h
index c0eb4dbf65f..51a1717ec62 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -684,7 +684,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
size_type
erase(const key_type& __x)
- { return _M_t.erase(__x); }
+ { return _M_t._M_erase_unique(__x); }
#if __cplusplus >= 201103L
// _GLIBCXX_RESOLVE_LIB_DEFECTS
diff --git a/libstdc++-v3/include/bits/stl_tree.h
b/libstdc++-v3/include/bits/stl_tree.h
index 6f470f04f6a..9e80d449c7e 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -1225,6 +1225,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
size_type
erase(const key_type& __x);
+ size_type
+ _M_erase_unique(const key_type& __x);
+
#if __cplusplus >= 201103L
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// DR 130. Associative erase should return an iterator.
@@ -2518,6 +2521,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return __old_size - size();
}
+ template
+typename _Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::size_type
+_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::
+_M_erase_unique(const _Key& __x)
+{
+ iterator __it = find(__x);
+ if (__it == end())
+ return 0;
+
+ _M_erase_aux(__it);
+ return 1;
+}
+
template
typename _Rb_tree<_Key, _Val, _KeyOfValue,
--
2.44.0
Re: [RFC PATCH v1] c: Do not warn about external declaration following inline definition
Hi
2023. október 30., hétfő 19:01 keltezéssel, Joseph Myers írta:
> On Sat, 28 Oct 2023, Barnabás Pőcze wrote:
>
> > An external declaration following an inline definition is not redundant
> > because it forces the compiler to emit an external definition for the
> > function.
> > That is,
> >
> > inline void f(void) { }
> > [extern] void f(void);
> >
> > should not trigger the
> >
> > redundant redeclaration of ...
> >
> > warning.
>
>
> This should add a testcase to the testsuite (that fails before and passes
> after the front-end change is made).
I did not want to commit more effort until I have some feedback.
I will most certainly add a test case if it turns out that the change
seems reasonable and has a chance of being accepted.
Regards,
Barnabás Pőcze
[RFC PATCH v1] c: Do not warn about external declaration following inline definition
An external declaration following an inline definition is not redundant
because it forces the compiler to emit an external definition for the function.
That is,
inline void f(void) { }
[extern] void f(void);
should not trigger the
redundant redeclaration of ...
warning.
gcc/c/ChangeLog:
* c-decl.cc (diagnose_mismatched_decls): Add new
case for Wredundant-decls suppression.
---
gcc/c/c-decl.cc | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 7a145bed281..e86f7950858 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -2592,7 +2592,12 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
&& TREE_ASM_WRITTEN (olddecl) && !TREE_ASM_WRITTEN (newdecl))
/* Don't warn about a variable definition following a declaration. */
&& !(VAR_P (newdecl)
- && DECL_INITIAL (newdecl) && !DECL_INITIAL (olddecl)))
+ && DECL_INITIAL (newdecl) && !DECL_INITIAL (olddecl))
+ /* Don't warn about external declaration following inline definition. */
+ && !(TREE_CODE (newdecl) == FUNCTION_DECL
+ && !DECL_INITIAL (newdecl) && !DECL_DECLARED_INLINE_P (newdecl)
+ && DECL_EXTERNAL (newdecl) && DECL_INITIAL (olddecl)
+ && DECL_DECLARED_INLINE_P (olddecl) && DECL_EXTERNAL (olddecl)))
{
warned = warning (OPT_Wredundant_decls, "redundant redeclaration of
%q+D",
newdecl);
--
2.42.0
