libstdc++: Avoid unnecessary copies in ranges::min/max [PR112349]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
Tested on x86_64-pc-linux-gnu, does this look OK for trunk and perhaps
14?
-- >8 --
Use a local reference for the (now possibly lifetime extended) result of
*__first to avoid making unnecessary copies of it.
PR libstdc++/112349
libstdc++-v3/ChangeLog:
* include/bits/ranges_algo.h (__min_fn::operator()): Turn local
object __tmp into a reference.
* include/bits/ranges_util.h (__max_fn::operator()): Likewise.
* testsuite/25_algorithms/max/constrained.cc (test04): New test.
* testsuite/25_algorithms/min/constrained.cc (test04): New test.
---
libstdc++-v3/include/bits/ranges_algo.h | 4 +--
libstdc++-v3/include/bits/ranges_util.h | 4 +--
.../25_algorithms/max/constrained.cc | 25 +++++++++++++++++++
.../25_algorithms/min/constrained.cc | 25 +++++++++++++++++++
4 files changed, 54 insertions(+), 4 deletions(-)
Comments
On Tue, 29 Oct 2024 at 13:20, Patrick Palka <ppalka@redhat.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk and perhaps
> 14?
OK for both, thanks.
>
> -- >8 --
>
> Use a local reference for the (now possibly lifetime extended) result of
> *__first to avoid making unnecessary copies of it.
>
> PR libstdc++/112349
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/ranges_algo.h (__min_fn::operator()): Turn local
> object __tmp into a reference.
> * include/bits/ranges_util.h (__max_fn::operator()): Likewise.
> * testsuite/25_algorithms/max/constrained.cc (test04): New test.
> * testsuite/25_algorithms/min/constrained.cc (test04): New test.
> ---
> libstdc++-v3/include/bits/ranges_algo.h | 4 +--
> libstdc++-v3/include/bits/ranges_util.h | 4 +--
> .../25_algorithms/max/constrained.cc | 25 +++++++++++++++++++
> .../25_algorithms/min/constrained.cc | 25 +++++++++++++++++++
> 4 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
> index bae36637b3e..e1aba256241 100644
> --- a/libstdc++-v3/include/bits/ranges_algo.h
> +++ b/libstdc++-v3/include/bits/ranges_algo.h
> @@ -2945,11 +2945,11 @@ namespace ranges
> auto __result = *__first;
> while (++__first != __last)
> {
> - auto __tmp = *__first;
> + auto&& __tmp = *__first;
> if (std::__invoke(__comp,
> std::__invoke(__proj, __result),
> std::__invoke(__proj, __tmp)))
> - __result = std::move(__tmp);
> + __result = std::forward<decltype(__tmp)>(__tmp);
> }
> return __result;
> }
> diff --git a/libstdc++-v3/include/bits/ranges_util.h b/libstdc++-v3/include/bits/ranges_util.h
> index 3f191e6d446..4a5349ae92a 100644
> --- a/libstdc++-v3/include/bits/ranges_util.h
> +++ b/libstdc++-v3/include/bits/ranges_util.h
> @@ -754,11 +754,11 @@ namespace ranges
> auto __result = *__first;
> while (++__first != __last)
> {
> - auto __tmp = *__first;
> + auto&& __tmp = *__first;
> if (std::__invoke(__comp,
> std::__invoke(__proj, __tmp),
> std::__invoke(__proj, __result)))
> - __result = std::move(__tmp);
> + __result = std::forward<decltype(__tmp)>(__tmp);
> }
> return __result;
> }
> diff --git a/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc
> index e7269e1b734..717900656bd 100644
> --- a/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc
> +++ b/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc
> @@ -73,10 +73,35 @@ test03()
> VERIFY( ranges::max({2,3,1,4}, ranges::greater{}, std::negate<>{}) == 4 );
> }
>
> +void
> +test04()
> +{
> + // PR libstdc++/112349 - ranges::max/min make unnecessary copies
> + static int copies, moves;
> + struct A {
> + A(int m) : m(m) { }
> + A(const A& other) : m(other.m) { ++copies; }
> + A(A&& other) : m(other.m) { ++moves; }
> + A& operator=(const A& other) { m = other.m; ++copies; return *this; }
> + A& operator=(A&& other) { m = other.m; ++moves; return *this; }
> + int m;
> + };
> + A r[5] = {5, 4, 3, 2, 1};
> + ranges::max(r, std::less{}, &A::m);
> + VERIFY( copies == 1 );
> + VERIFY( moves == 0 );
> + copies = moves = 0;
> + A s[5] = {1, 2, 3, 4, 5};
> + ranges::max(s, std::less{}, &A::m);
> + VERIFY( copies == 5 );
> + VERIFY( moves == 0 );
> +}
> +
> int
> main()
> {
> test01();
> test02();
> test03();
> + test04();
> }
> diff --git a/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc
> index 7198df69adf..d338a86f186 100644
> --- a/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc
> +++ b/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc
> @@ -73,10 +73,35 @@ test03()
> VERIFY( ranges::min({2,3,1,4}, ranges::greater{}, std::negate<>{}) == 1 );
> }
>
> +void
> +test04()
> +{
> + // PR libstdc++/112349 - ranges::max/min make unnecessary copies
> + static int copies, moves;
> + struct A {
> + A(int m) : m(m) { }
> + A(const A& other) : m(other.m) { ++copies; }
> + A(A&& other) : m(other.m) { ++moves; }
> + A& operator=(const A& other) { m = other.m; ++copies; return *this; }
> + A& operator=(A&& other) { m = other.m; ++moves; return *this; }
> + int m;
> + };
> + A r[5] = {5, 4, 3, 2, 1};
> + ranges::min(r, std::less{}, &A::m);
> + VERIFY( copies == 5 );
> + VERIFY( moves == 0 );
> + copies = moves = 0;
> + A s[5] = {1, 2, 3, 4, 5};
> + ranges::min(s, std::less{}, &A::m);
> + VERIFY( copies == 1 );
> + VERIFY( moves == 0 );
> +}
> +
> int
> main()
> {
> test01();
> test02();
> test03();
> + test04();
> }
> --
> 2.47.0.148.g6a11438f43
>
@@ -2945,11 +2945,11 @@ namespace ranges
auto __result = *__first;
while (++__first != __last)
{
- auto __tmp = *__first;
+ auto&& __tmp = *__first;
if (std::__invoke(__comp,
std::__invoke(__proj, __result),
std::__invoke(__proj, __tmp)))
- __result = std::move(__tmp);
+ __result = std::forward<decltype(__tmp)>(__tmp);
}
return __result;
}
@@ -754,11 +754,11 @@ namespace ranges
auto __result = *__first;
while (++__first != __last)
{
- auto __tmp = *__first;
+ auto&& __tmp = *__first;
if (std::__invoke(__comp,
std::__invoke(__proj, __tmp),
std::__invoke(__proj, __result)))
- __result = std::move(__tmp);
+ __result = std::forward<decltype(__tmp)>(__tmp);
}
return __result;
}
@@ -73,10 +73,35 @@ test03()
VERIFY( ranges::max({2,3,1,4}, ranges::greater{}, std::negate<>{}) == 4 );
}
+void
+test04()
+{
+ // PR libstdc++/112349 - ranges::max/min make unnecessary copies
+ static int copies, moves;
+ struct A {
+ A(int m) : m(m) { }
+ A(const A& other) : m(other.m) { ++copies; }
+ A(A&& other) : m(other.m) { ++moves; }
+ A& operator=(const A& other) { m = other.m; ++copies; return *this; }
+ A& operator=(A&& other) { m = other.m; ++moves; return *this; }
+ int m;
+ };
+ A r[5] = {5, 4, 3, 2, 1};
+ ranges::max(r, std::less{}, &A::m);
+ VERIFY( copies == 1 );
+ VERIFY( moves == 0 );
+ copies = moves = 0;
+ A s[5] = {1, 2, 3, 4, 5};
+ ranges::max(s, std::less{}, &A::m);
+ VERIFY( copies == 5 );
+ VERIFY( moves == 0 );
+}
+
int
main()
{
test01();
test02();
test03();
+ test04();
}
@@ -73,10 +73,35 @@ test03()
VERIFY( ranges::min({2,3,1,4}, ranges::greater{}, std::negate<>{}) == 1 );
}
+void
+test04()
+{
+ // PR libstdc++/112349 - ranges::max/min make unnecessary copies
+ static int copies, moves;
+ struct A {
+ A(int m) : m(m) { }
+ A(const A& other) : m(other.m) { ++copies; }
+ A(A&& other) : m(other.m) { ++moves; }
+ A& operator=(const A& other) { m = other.m; ++copies; return *this; }
+ A& operator=(A&& other) { m = other.m; ++moves; return *this; }
+ int m;
+ };
+ A r[5] = {5, 4, 3, 2, 1};
+ ranges::min(r, std::less{}, &A::m);
+ VERIFY( copies == 5 );
+ VERIFY( moves == 0 );
+ copies = moves = 0;
+ A s[5] = {1, 2, 3, 4, 5};
+ ranges::min(s, std::less{}, &A::m);
+ VERIFY( copies == 1 );
+ VERIFY( moves == 0 );
+}
+
int
main()
{
test01();
test02();
test03();
+ test04();
}