[Bug libstdc++/109383] New: [QoI] std::type_index::operator<=> should not call __builtin_strcmp twice

[Bug libstdc++/109383] New: [QoI] std::type_index::operator<=> should not call __builtin_strcmp twice

[de34 at live dot cn]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109383

            Bug ID: 109383
           Summary: [QoI] std::type_index::operator<=> should not call
                    __builtin_strcmp twice
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: de34 at live dot cn
  Target Milestone: ---

The current the current implementation of type_index::operator<=> in libstdc++
is the following:
```
    strong_ordering
    operator<=>(const type_index& __rhs) const noexcept
    {
      if (*_M_target == *__rhs._M_target)
        return strong_ordering::equal;
      if (_M_target->before(*__rhs._M_target))
        return strong_ordering::less;
      return strong_ordering::greater;
    }
```

When the two referenced type_info are not equal, the current implementation may
needed to call __builtin_strcmp twice (each in type_info::operator== and
type_info::before).

IIUC whenever it's necessary to call __builtin_strcmp from operator<=>, we
should just call it once and return __builtin_strcmp(...) <=> 0. Perhaps it
would be better to add a member function to type_info for convenience.


It's a bit strange to me that while all implementations I investigated (libc++,
libstdc++, and msvc stl) need to use strcmp/__builtin_strcmp for type_info
comparison in some cases, currently none of them avoids calling it twice from
type_index::operator<=>, although strcmp/__builtin_strcmp is already performing
three-way comparison.

[Bug libstdc++/109383] [QoI] std::type_index::operator<=> should not call __builtin_strcmp twice

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109383

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-04-03

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I guess we've all just implemented exactly what the standard says, without
thinking about optimizing it. It's a good idea though, thanks!

[Bug libstdc++/109383] [QoI] std::type_index::operator<=> should not call __builtin_strcmp twice

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109383

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
--- a/libstdc++-v3/include/std/typeindex
+++ b/libstdc++-v3/include/std/typeindex
@@ -87,13 +87,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     strong_ordering
     operator<=>(const type_index& __rhs) const noexcept
     {
+#if __GXX_TYPEINFO_EQUALITY_INLINE
+# if __GXX_MERGED_TYPEINFO_NAMES
+      return _M_target->name() <=> __rhs._M_target->name();
+# else
+      return __builtin_strcmp(_M_target->name(), __rhs._M_target->name()) <=>
0;
+# endif
+#else // __GXX_TYPEINFO_EQUALITY_INLINE
       if (*_M_target == *__rhs._M_target)
        return strong_ordering::equal;
       if (_M_target->before(*__rhs._M_target))
        return strong_ordering::less;
       return strong_ordering::greater;
+#endif // __GXX_TYPEINFO_EQUALITY_INLINE
     }
-#endif
+#endif // __cpp_lib_three_way_comparison

     size_t
     hash_code() const noexcept

[Bug libstdc++/109383] [QoI] std::type_index::operator<=> should not call __builtin_strcmp twice

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109383

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
For !__GXX_TYPEINFO_EQUALITY_INLINE we need to export a new symbol from the
library, so the patch above doesn't optimize that case.

[Bug libstdc++/109383] [QoI] std::type_index::operator<=> should not call __builtin_strcmp twice

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109383

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement