[libc++] Avoid constructing additional objects when using map::at#157866
Merged
philnik777 merged 1 commit intollvm:mainfrom Sep 25, 2025
Merged
[libc++] Avoid constructing additional objects when using map::at#157866philnik777 merged 1 commit intollvm:mainfrom
philnik777 merged 1 commit intollvm:mainfrom
Conversation
philnik777
changed the title
philnik777
force-pushed
the
map_at_key_extraction
branch
12 times, most recently
from
1000e67 to
c7c7468
Compare
llvmbot
added
the
libc++
Member
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesThis patch adds additional overloads to 8 Files Affected:
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index db918a16e9a61..fed0141daee81 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -862,6 +862,7 @@ set(files
__type_traits/is_specialization.h
__type_traits/is_standard_layout.h
__type_traits/is_swappable.h
+ __type_traits/is_transparently_comparable.h
__type_traits/is_trivial.h
__type_traits/is_trivially_assignable.h
__type_traits/is_trivially_constructible.h
@@ -880,6 +881,7 @@ set(files
__type_traits/make_32_64_or_128_bit.h
__type_traits/make_const_lvalue_ref.h
__type_traits/make_signed.h
+ __type_traits/make_transparent.h
__type_traits/make_unsigned.h
__type_traits/maybe_const.h
__type_traits/nat.h
diff --git a/libcxx/include/__functional/operations.h b/libcxx/include/__functional/operations.h
index 7b0ea11db5844..cee10aea14236 100644
--- a/libcxx/include/__functional/operations.h
+++ b/libcxx/include/__functional/operations.h
@@ -16,6 +16,7 @@
#include <__fwd/functional.h>
#include <__type_traits/desugars_to.h>
#include <__type_traits/is_integral.h>
+#include <__type_traits/make_transparent.h>
#include <__utility/forward.h>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -377,6 +378,11 @@ struct less<void> {
typedef void is_transparent;
};
+template <class _Tp>
+struct __make_transparent<less<_Tp> > {
+ using type _LIBCPP_NODEBUG = less<>;
+};
+
template <class _Tp, class _Up>
inline const bool __desugars_to_v<__less_tag, less<>, _Tp, _Up> = true;
@@ -466,6 +472,11 @@ struct greater<void> {
template <class _Tp, class _Up>
inline const bool __desugars_to_v<__greater_tag, greater<>, _Tp, _Up> = true;
+
+template <class _Tp>
+struct __make_transparent<greater<_Tp>> {
+ using type _LIBCPP_NODEBUG = greater<>;
+};
#endif
// Logical operations
diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index 61c910c52c536..ef960d481cb7b 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -34,6 +34,7 @@
#include <__type_traits/is_same.h>
#include <__type_traits/is_specialization.h>
#include <__type_traits/is_swappable.h>
+#include <__type_traits/make_transparent.h>
#include <__type_traits/remove_const.h>
#include <__utility/forward.h>
#include <__utility/lazy_synth_three_way_comparator.h>
@@ -1749,7 +1750,8 @@ __tree<_Tp, _Compare, _Allocator>::__find_equal(const _Key& __v) {
}
__node_base_pointer* __node_ptr = __root_ptr();
- auto __comp = __lazy_synth_three_way_comparator<_Compare, _Key, value_type>(value_comp());
+ auto&& __transparent = std::__as_transparent(value_comp());
+ auto __comp = __lazy_synth_three_way_comparator<__make_transparent_t<_Compare>, _Key, value_type>(__transparent);
while (true) {
auto __comp_res = __comp(__v, __nd->__get_value());
diff --git a/libcxx/include/__type_traits/is_transparently_comparable.h b/libcxx/include/__type_traits/is_transparently_comparable.h
new file mode 100644
index 0000000000000..9e27c485360fd
--- /dev/null
+++ b/libcxx/include/__type_traits/is_transparently_comparable.h
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___TYPE_TRAITS_IS_TRANSPARENTLY_COMPARABLE_H
+#define _LIBCPP___TYPE_TRAITS_IS_TRANSPARENTLY_COMPARABLE_H
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template <class _Key, class _Arg>
+inline const bool __is_transparently_comparable_v = __is_same(_Key, _Arg);
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___TYPE_TRAITS_IS_TRANSPARENTLY_COMPARABLE_H
diff --git a/libcxx/include/__type_traits/make_transparent.h b/libcxx/include/__type_traits/make_transparent.h
new file mode 100644
index 0000000000000..9aa247d284707
--- /dev/null
+++ b/libcxx/include/__type_traits/make_transparent.h
@@ -0,0 +1,42 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___TYPE_TRAITS_MAKE_TRANSPARENT_H
+#define _LIBCPP___TYPE_TRAITS_MAKE_TRANSPARENT_H
+
+#include <__config>
+#include <__type_traits/enable_if.h>
+#include <__type_traits/is_same.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template <class _Comparator>
+struct __make_transparent {
+ using type _LIBCPP_NODEBUG = _Comparator;
+};
+
+template <class _Comparator>
+using __make_transparent_t _LIBCPP_NODEBUG = typename __make_transparent<_Comparator>::type;
+
+template <class _Comparator, __enable_if_t<is_same<_Comparator, __make_transparent_t<_Comparator> >::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _Comparator& __as_transparent(_Comparator& __comp) {
+ return __comp;
+}
+
+template <class _Comparator, __enable_if_t<!is_same<_Comparator, __make_transparent_t<_Comparator> >::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI __make_transparent_t<_Comparator> __as_transparent(_Comparator&) {
+ return __make_transparent_t<_Comparator>();
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___TYPE_TRAITS_MAKE_TRANSPARENT_H
diff --git a/libcxx/include/map b/libcxx/include/map
index f428c781e5036..cb7a5120a950f 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -599,7 +599,11 @@ erase_if(multimap<Key, T, Compare, Allocator>& c, Predicate pred); // C++20
# include <__ranges/from_range.h>
# include <__tree>
# include <__type_traits/container_traits.h>
+# include <__type_traits/desugars_to.h>
# include <__type_traits/is_allocator.h>
+# include <__type_traits/is_convertible.h>
+# include <__type_traits/is_transparently_comparable.h>
+# include <__type_traits/make_transparent.h>
# include <__type_traits/remove_const.h>
# include <__type_traits/type_identity.h>
# include <__utility/forward.h>
@@ -703,6 +707,11 @@ public:
# endif
};
+template <class _Key, class _MapValueT, class _Compare, bool __b>
+struct __make_transparent<__map_value_compare<_Key, _MapValueT, _Compare, __b> > {
+ using type _LIBCPP_NODEBUG = __map_value_compare<_Key, _MapValueT, __make_transparent_t<_Compare> >;
+};
+
# if _LIBCPP_STD_VER >= 14
template <class _MapValueT, class _Key, class _Compare>
struct __lazy_synth_three_way_comparator<__map_value_compare<_Key, _MapValueT, _Compare>, _MapValueT, _MapValueT> {
@@ -1085,6 +1094,30 @@ public:
_LIBCPP_HIDE_FROM_ABI mapped_type& operator[](key_type&& __k);
# endif
+ template <class _Arg,
+ __enable_if_t<is_convertible<_Arg, value_type>::value &&
+ __desugars_to_v<__less_tag, key_compare, key_type, key_type> &&
+ __is_transparently_comparable_v<key_type, __remove_cvref_t<_Arg> >,
+ int> = 0>
+ _LIBCPP_HIDE_FROM_ABI inline mapped_type& at(_Arg&& __arg) {
+ auto [_, __child] = __tree_.__find_equal(__arg);
+ if (__child == nullptr)
+ std::__throw_out_of_range("map::at: key not found");
+ return static_cast<__node_pointer>(__child)->__get_value().second;
+ }
+
+ template <class _Arg,
+ __enable_if_t<is_convertible<_Arg, value_type>::value &&
+ __desugars_to_v<__less_tag, key_compare, key_type, key_type> &&
+ __is_transparently_comparable_v<key_type, __remove_cvref_t<_Arg> >,
+ int> = 0>
+ _LIBCPP_HIDE_FROM_ABI inline const mapped_type& at(_Arg&& __arg) const {
+ auto [_, __child] = __tree_.__find_equal(__arg);
+ if (__child == nullptr)
+ std::__throw_out_of_range("map::at: key not found");
+ return static_cast<__node_pointer>(__child)->__get_value().second;
+ }
+
_LIBCPP_HIDE_FROM_ABI mapped_type& at(const key_type& __k);
_LIBCPP_HIDE_FROM_ABI const mapped_type& at(const key_type& __k) const;
diff --git a/libcxx/include/module.modulemap.in b/libcxx/include/module.modulemap.in
index 63cf8e847751f..401551bdb8eaa 100644
--- a/libcxx/include/module.modulemap.in
+++ b/libcxx/include/module.modulemap.in
@@ -299,6 +299,7 @@ module std_core [system] {
header "__type_traits/is_swappable.h"
export std_core.type_traits.integral_constant
}
+ module is_transparently_comparable { header "__type_traits/is_transparently_comparable.h" }
module is_trivial {
header "__type_traits/is_trivial.h"
export std_core.type_traits.integral_constant
@@ -356,6 +357,7 @@ module std_core [system] {
module make_32_64_or_128_bit { header "__type_traits/make_32_64_or_128_bit.h" }
module make_const_lvalue_ref { header "__type_traits/make_const_lvalue_ref.h" }
module make_signed { header "__type_traits/make_signed.h" }
+ module make_transparent { header "__type_traits/make_transparent.h" }
module make_unsigned { header "__type_traits/make_unsigned.h" }
module maybe_const { header "__type_traits/maybe_const.h" }
module nat { header "__type_traits/nat.h" }
diff --git a/libcxx/include/string b/libcxx/include/string
index f13a7640760f7..9e80c8f8acc43 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -633,6 +633,7 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
# include <__type_traits/is_replaceable.h>
# include <__type_traits/is_same.h>
# include <__type_traits/is_standard_layout.h>
+# include <__type_traits/is_transparently_comparable.h>
# include <__type_traits/is_trivially_constructible.h>
# include <__type_traits/is_trivially_copyable.h>
# include <__type_traits/is_trivially_relocatable.h>
@@ -2536,6 +2537,16 @@ struct __default_three_way_comparator<basic_string<_CharT, _Traits, _Alloc>, bas
};
# endif
+template <class _CharT, class _Traits, class _Alloc>
+inline const bool
+ __is_transparently_comparable_v<basic_string<_CharT, _Traits, _Alloc>, basic_string_view<_CharT, _Traits> > = true;
+
+template <class _CharT, class _Traits, class _Alloc>
+inline const bool __is_transparently_comparable_v<basic_string<_CharT, _Traits, _Alloc>, const _CharT*> = true;
+
+template <class _CharT, class _Traits, class _Alloc, size_t _Np>
+inline const bool __is_transparently_comparable_v<basic_string<_CharT, _Traits, _Alloc>, _CharT[_Np]> = true;
+
# if _LIBCPP_STD_VER >= 17
template <class _InputIterator,
class _CharT = __iter_value_type<_InputIterator>,
|
ldionne
requested changes
Sep 15, 2025
Member
ldionne
left a comment
ldionne
left a comment
There was a problem hiding this comment.
Let's also add a benchmark with e.g. map.at(string literal)
philnik777
force-pushed
the
map_at_key_extraction
branch
2 times, most recently
from
0c65e8a to
0047709
Compare
ldionne
reviewed
Sep 17, 2025
Member
ldionne
left a comment
ldionne
left a comment
There was a problem hiding this comment.
General comment: let's write a benchmark for this.
Comment on lines
+1309
to
+1310
| template <typename _K2, | ||
| enable_if_t<__is_transparent_v<_Compare, _K2> || __is_transparently_comparable_v<_Compare, key_type, _K2>, |
Member
There was a problem hiding this comment.
This is very confusing as-is. The only reason for passing _K2 to __is_transparent_v seems to be to make it dependent. The right way to achieve that is usually:
template <class _K2, class _DependentCompare = _Compare, enable_if_t<__is_transparent_v<_DependentCompare>>, ...>If we did that, then we could get rid of the other arguments of __is_transparent_v, and that would make this code a lot clearer IMO, since we'd easily understand that __is_transparent_v only checks the typedef.
Let's do that as a follow-up refactoring, or as a prerequisite for this patch.
Member
There was a problem hiding this comment.
Not attached to this file: let's do the same optimization in unordered_map,in a followup.
philnik777
force-pushed
the
map_at_key_extraction
branch
from
0047709 to
91b3719
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
philnik777
force-pushed
the
map_at_key_extraction
branch
2 times, most recently
from
56c9721 to
b80dc5f
Compare
ldionne
approved these changes
Sep 23, 2025
philnik777
force-pushed
the
map_at_key_extraction
branch
from
b80dc5f to
a1b7333
Compare
philnik777
force-pushed
the
map_at_key_extraction
branch
from
a1b7333 to
350a9d5
Compare
Member
|
Please note that this patch has broken lldb-remote-linux-ubuntu. |
Contributor
|
Some specializations in LLVM itself don't look correct. Opened #160684 to track this. |
ckoparkar
added a commit
to ckoparkar/llvm-project
that referenced
this pull request
Sep 25, 2025
* main: (502 commits) GlobalISel: Adjust insert point when expanding G_[SU]DIVREM (llvm#160683) [LV] Add coverage for fixing-up scalar resume values (llvm#160492) AMDGPU: Convert wave_any test to use update_mc_test_checks [LV] Add partial reduction tests multiplying extend with constants. Revert "[MLIR] Implement remark emitting policies in MLIR" (llvm#160681) [NFC][InstSimplify] Refactor fminmax-folds.ll test (llvm#160504) [LoongArch] Pre-commit tests for [x]vldi instructions with special constant splats (llvm#159228) [BOLT] Fix dwarf5-dwoid-no-dwoname.s (llvm#160676) [lldb][test] Refactor and expand TestMemoryRegionDirtyPages.py (llvm#156035) [gn build] Port 833d5f0 AMDGPU: Ensure both wavesize features are not set (llvm#159234) [LoopInterchange] Bail out when finding a dependency with all `*` elements (llvm#149049) [libc++] Avoid constructing additional objects when using map::at (llvm#157866) [lldb][test] Make hex prefix optional in DWARF union types test [X86] Add missing prefixes to trunc-sat tests (llvm#160662) [AMDGPU] Fix vector legalization for bf16 valu ops (llvm#158439) [LoongArch][NFC] Pre-commit tests for `[x]vadda.{b/h/w/d}` [mlir][tosa] Relax constraint on matmul verifier requiring equal operand types (llvm#155799) [clang][Sema] Accept gnu format attributes (llvm#160255) [LoongArch][NFC] Add tests for element extraction from binary add operation (llvm#159725) ...
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/15215 Here is the relevant piece of the build log for the reference |
Contributor
Author
|
As Frederick already said, these specializations are definitely non-conforming. I guess users would be allowed to specialize That' being said, I'm fine with reverting temporarily if it takes a bit of time to resolve the issue. |
Contributor
Collaborator
|
This change is currently breaking the emscripten release builders too: I suggest we land the revert |
llvm-sync Bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Sep 25, 2025
…n using map::at" (#160738) Reverts llvm/llvm-project#157866 It breaks a lot of sanitizer buildbots
Contributor
|
I think we can reland this now. |
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
…vm#157866) This patch adds additional overloads to `map::at` in case its known that the argument is transparently comparable to the key type. This avoids actually constructing the key type in some cases, potentially removing allocations. ``` -------------------------------------------------------- Benchmark old new -------------------------------------------------------- BM_map_find_string_literal 12.8 ns 2.68 ns ```
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
…::at" (llvm#160738) Reverts llvm#157866 It breaks a lot of sanitizer buildbots
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch adds additional overloads to
map::atin case its known that the argument is transparently comparable to the key type. This avoids actually constructing the key type in some cases, potentially removing allocations.