[clang] [libcxx] Unifying __is_trivial_equality_predicate and __is_trivial_plus_operation into __desugars_to (PR #68642)

Louis Dionne via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 13 17:54:05 PDT 2023


================
@@ -41,13 +42,12 @@ _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 boo
   return true;
 }
 
-template <
-    class _Tp,
-    class _Up,
-    class _BinaryPredicate,
-    __enable_if_t<__is_trivial_equality_predicate<_BinaryPredicate, _Tp, _Up>::value && !is_volatile<_Tp>::value &&
-                      !is_volatile<_Up>::value && __libcpp_is_trivially_equality_comparable<_Tp, _Up>::value,
-                  int> = 0>
+template < class _Tp,
+           class _Up,
+           class _BinaryPredicate,
+           __enable_if_t<__desugars_to<_BinaryPredicate, equal_to<_Tp> >::value && !is_volatile<_Tp>::value &&
----------------
ldionne wrote:

I think it is pretty important to say `__desugars_to<_BinaryPredicate, equal_to<void> >::value` here. `equal_to<void>` represents the "transparent" version of `equal_to`, i.e. the one that does `x == y` without any conversion involved. If you use `__desugars_to<_BinaryPredicate, equal_to<_Tp> >::value`, it means that if `_Up` and `_Tp` are different but they happen to be trivially equality-comparable, we would skip this optimization.

You should be able to check that this optimization gets disabled if you use something like [this](https://godbolt.org/z/hGq57j5jM):

```
#include <algorithm>

using T = int*;
using U = void*;

bool f(T* first1, T* last1, U* first2, U* last2) {
    return std::equal(first1, last1, first2);
}
```

```
f(int**, int**, void**, void**):                       # @f(int**, int**, void**, void**)
        push    rax
        mov     rax, rsi
        sub     rax, rdi
        mov     rsi, rdx
        mov     rdx, rax
        call    bcmp at PLT
        test    eax, eax
        sete    al
        pop     rcx
        ret
```

This is good, if there's no `bcmp` or `memcmp` in the output then the optimization was disabled. Since we don't have access to FileCheck in the libc++ test suite, however, it's going to be really hard to add a test for this. But it would be great if you can validate it manually at least. You can do something like this:

```
ninja -C build cxx-test-depends

cat <<EOF | clang++ -xc++ - -nostdinc++ -isystem build/default/include/c++/v1 -std=c++17 -S -o a.s -O3 && cat a.s
#include <algorithm>

using T = int*;
using U = void*;

bool f(T* first1, T* last1, U* first2, U* last2) {
    return std::equal(first1, last1, first2);
}
EOF
```

https://github.com/llvm/llvm-project/pull/68642


More information about the cfe-commits mailing list