[libcxx-commits] [libcxx] 17095dc - [libc++][NFC] Increase readability of typeinfo comparison of ARM64

[Louis Dionne via libcxx-commits]

Author: Louis Dionne
Date: 2021-04-01T16:38:34-04:00
New Revision: 17095dc861116b0fe4303b62d1fed5eb14c2ecd0

URL: https://github.com/llvm/llvm-project/commit/17095dc861116b0fe4303b62d1fed5eb14c2ecd0
DIFF: https://github.com/llvm/llvm-project/commit/17095dc861116b0fe4303b62d1fed5eb14c2ecd0.diff

LOG: [libc++][NFC] Increase readability of typeinfo comparison of ARM64

We wasted a good deal of time trying to figure out whether our implementation
was correct. In the end, it was, but it wasn't so easy to determine. This
patch dumbs down the implementation and improves the documentation to make
it easier to validate.

See https://lists.llvm.org/pipermail/libcxx-dev/2020-December/001060.html.

Differential Revision: https://reviews.llvm.org/D97802

Added: 
    

Modified: 
    libcxx/include/typeinfo

Removed: 
    


################################################################################
diff  --git a/libcxx/include/typeinfo b/libcxx/include/typeinfo
index 048e57614aff..966cc9210cc5 100644
--- a/libcxx/include/typeinfo
+++ b/libcxx/include/typeinfo
@@ -125,7 +125,7 @@ public:
 //               (_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION = 1)
 // ------------------------------------------------------------------------- //
 // This implementation of type_info assumes a unique copy of the RTTI for a
-// given type inside a program. This is a valid assumption when abiding to
+// given type inside a program. This is a valid assumption when abiding to the
 // Itanium ABI (http://itanium-cxx-abi.github.io/cxx-abi/abi.html#vtable-components).
 // Under this assumption, we can always compare the addresses of the type names
 // to implement equality-comparison of type_infos instead of having to perform
@@ -144,22 +144,29 @@ public:
 //                          NonUniqueARMRTTIBit
 //               (_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION = 3)
 // -------------------------------------------------------------------------- //
+// This implementation is specific to ARM64 on Apple platforms.
+//
 // This implementation of type_info does not assume always a unique copy of
-// the RTTI for a given type inside a program. It packs the pointer to the
-// type name into a uintptr_t and reserves the high bit of that pointer (which
-// is assumed to be free for use under the ABI in use) to represent whether
-// that specific copy of the RTTI can be assumed unique inside the program.
-// To implement equality-comparison of type_infos, we check whether BOTH
-// type_infos are guaranteed unique, and if so, we simply compare the addresses
-// of their type names instead of doing a deep string comparison, which is
-// faster. If at least one of the type_infos can't guarantee uniqueness, we
-// have no choice but to fall back to a deep string comparison.
+// the RTTI for a given type inside a program. When constructing the type_info,
+// the compiler packs the pointer to the type name into a uintptr_t and reserves
+// the high bit of that pointer, which is assumed to be free for use under that
+// ABI. If that high bit is set, that specific copy of the RTTI can't be assumed
+// to be unique within the program. If the high bit is unset, then the RTTI can
+// be assumed to be unique within the program.
 //
-// This implementation is specific to ARM64 on Apple platforms.
+// When comparing type_infos, if both RTTIs can be assumed to be unique, it
+// suffices to compare their addresses. If both the RTTIs can't be assumed to
+// be unique, we must perform a deep string comparison of the type names.
+// However, if one of the RTTIs is guaranteed unique and the other one isn't,
+// then both RTTIs are necessarily not to be considered equal.
 //
-// Note that the compiler is the one setting (or unsetting) the high bit of
-// the pointer when it constructs the type_info, depending on whether it can
-// guarantee uniqueness for that specific type_info.
+// The intent of this design is to remove the need for weak symbols. Specifically,
+// if a type would normally have a default-visibility RTTI emitted as a weak
+// symbol, it is given hidden visibility instead and the non-unique bit is set.
+// Otherwise, types declared with hidden visibility are always considered to have
+// a unique RTTI: the RTTI is emitted with linkonce_odr linkage and is assumed
+// to be deduplicated by the linker within the linked image. Across linked image
+// boundaries, such types are thus considered 
diff erent types.
 
 // This value can be overriden in the __config_site. When it's not overriden,
 // we pick a default implementation based on the platform here.
@@ -248,13 +255,15 @@ struct __type_info_implementations {
     static bool __eq(__type_name_t __lhs, __type_name_t __rhs) _NOEXCEPT {
       if (__lhs == __rhs)
         return true;
-      if (__is_type_name_unique(__lhs, __rhs))
+      if (__is_type_name_unique(__lhs) || __is_type_name_unique(__rhs))
+        // Either both are unique and have a 
diff erent address, or one of them
+        // is unique and the other one isn't. In both cases they are unequal.
         return false;
       return __builtin_strcmp(__type_name_to_string(__lhs), __type_name_to_string(__rhs)) == 0;
     }
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_ALWAYS_INLINE
     static bool __lt(__type_name_t __lhs, __type_name_t __rhs) _NOEXCEPT {
-      if (__is_type_name_unique(__lhs, __rhs))
+      if (__is_type_name_unique(__lhs) || __is_type_name_unique(__rhs))
         return __lhs < __rhs;
       return __builtin_strcmp(__type_name_to_string(__lhs), __type_name_to_string(__rhs)) < 0;
     }
@@ -269,10 +278,6 @@ struct __type_info_implementations {
     static bool __is_type_name_unique(__type_name_t __lhs) _NOEXCEPT {
       return !(__lhs & __non_unique_rtti_bit::value);
     }
-    _LIBCPP_INLINE_VISIBILITY
-    static bool __is_type_name_unique(__type_name_t __lhs, __type_name_t __rhs) _NOEXCEPT {
-      return !((__lhs & __rhs) & __non_unique_rtti_bit::value);
-    }
   };
 
   typedef