Skip to content

Conversation

@philnik777
Copy link
Contributor

If the string is too long for a short string, we can simply check for the long bit. If that's false we can do an early return. This improves the code gen slightly.

@philnik777 philnik777 requested a review from a team as a code owner July 28, 2024 11:51
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 28, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2024

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

If the string is too long for a short string, we can simply check for the long bit. If that's false we can do an early return. This improves the code gen slightly.


Full diff: https://github.com/llvm/llvm-project/pull/100926.diff

1 Files Affected:

  • (modified) libcxx/include/string (+11-5)
diff --git a/libcxx/include/string b/libcxx/include/string
index 9fa979e3a5178..856e422d8c1e3 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2246,6 +2246,10 @@ private:
   friend constexpr basic_string operator+ <>(const basic_string&, type_identity_t<__self_view>);
   friend constexpr basic_string operator+ <>(type_identity_t<__self_view>, const basic_string&);
 #endif
+
+  template <class _CharT2, class _Traits2, class _Allocator2>
+  friend inline _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool
+  operator==(const basic_string<_CharT2, _Traits2, _Allocator2>&, const _CharT2*) _NOEXCEPT;
 };
 
 // These declarations must appear before any functions are implicitly used
@@ -3855,16 +3859,18 @@ operator==(const _CharT* __lhs, const basic_string<_CharT, _Traits, _Allocator>&
 template <class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool
 operator==(const basic_string<_CharT, _Traits, _Allocator>& __lhs, const _CharT* __rhs) _NOEXCEPT {
-#if _LIBCPP_STD_VER >= 20
-  return basic_string_view<_CharT, _Traits>(__lhs) == basic_string_view<_CharT, _Traits>(__rhs);
-#else
-  typedef basic_string<_CharT, _Traits, _Allocator> _String;
   _LIBCPP_ASSERT_NON_NULL(__rhs != nullptr, "operator==(basic_string, char*): received nullptr");
+
+  using _String = basic_string<_CharT, _Traits, _Allocator>;
+
   size_t __rhs_len = _Traits::length(__rhs);
+  if (__builtin_constant_p(__rhs_len) && __rhs_len >= _String::__min_cap) {
+    if (!__lhs.__is_long())
+      return false;
+  }
   if (__rhs_len != __lhs.size())
     return false;
   return __lhs.compare(0, _String::npos, __rhs, __rhs_len) == 0;
-#endif
 }
 
 #if _LIBCPP_STD_VER >= 20

@philnik777 philnik777 force-pushed the improve_string_cmpeq branch from 9d2d2dd to 12afc0e Compare July 28, 2024 11:56
Comment on lines +2250 to +2252
template <class _CharT2, class _Traits2, class _Allocator2>
friend inline _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool
operator==(const basic_string<_CharT2, _Traits2, _Allocator2>&, const _CharT2*) _NOEXCEPT;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not define the operator here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we wouldn't be conforming then. This operator== isn't defined as a hidden friend in the standard.

Comment on lines -3858 to -3860
#if _LIBCPP_STD_VER >= 20
return basic_string_view<_CharT, _Traits>(__lhs) == basic_string_view<_CharT, _Traits>(__rhs);
#else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the purpose of this #if?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT it only exists to match the standards wording.

@philnik777 philnik777 merged commit 3af26be into llvm:main Aug 1, 2024
@philnik777 philnik777 deleted the improve_string_cmpeq branch August 1, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants