From 9c03935c9a22f3f6baa602ec0ea7ce91f4a842f0 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 16 Nov 2023 10:09:12 +0100 Subject: [PATCH] Qt::_*ordering: de-pessimize conversion to std types Instead of an if-else-if chain, use a switch and Q_UNREACHABLE_RETURN. We don't need the GCC-8 workaround for Q_UNREACHABLE_RETURN in constexpr here as GCC-8 doesn't have , yet, so it will never compile this code. Using this test: extern Qt::partial_ordering qt_partial_ordering() noexcept; std::partial_ordering qt2std() { return qt_partial_ordering(); } I see these codegen improvements: Clang -O2 before: qt2std(): # @qt2std() pushq %rax callq qt_partial_ordering()@PLT testb %al, %al je .LBB36_3 movzbl %al, %ecx cmpl 55, %ecx jne .LBB36_4 popq %rcx retq .LBB36_3: xorl %eax, %eax popq %rcx retq .LBB36_4: cmpb , %al sete %cl movb , %al subb %cl, %al popq %rcx retq Clang -O2 now: qt2std(): jmp qt_partial_ordering()@PLT $# TAIL-CALL GCC -O2 before: qt2std(): subq $8, %rsp call qt_partial_ordering()@PLT cmpb $-1, %al je .L194 testb %al, %al jne .L203 .L194: addq $8, %rsp ret .L203: cmpb $1, %al movl $2, %edx cmovne %edx, %eax addq $8, %rsp ret GCC -O2 now: qt2std(): subq $8, %rsp call qt_partial_ordering()@PLT cmpb $-1, %al je .L194 cmpb $1, %al je .L194 xorl %edx, %edx cmpb $2, %al cmovne %edx, %eax .L194: addq $8, %rsp ret GCC needs -O3 to produce now: qt2std(): subq $8, %rsp call qt_partial_ordering()@PLT addq $8, %rsp Change-Id: Ic8c57c6e1bc39e247999b45dc263a8a8aef07dd0 Reviewed-by: Thiago Macieira Reviewed-by: Ivan Solovev --- src/corelib/global/qcompare.h | 44 +++++++++++++++-------------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/src/corelib/global/qcompare.h b/src/corelib/global/qcompare.h index 90341270d7..e01751ce11 100644 --- a/src/corelib/global/qcompare.h +++ b/src/corelib/global/qcompare.h @@ -266,15 +266,13 @@ public: constexpr Q_IMPLICIT operator std::partial_ordering() const noexcept { - if (static_cast(m_order) == QtPrivate::Ordering::Less) - return std::partial_ordering::less; - else if (static_cast(m_order) == QtPrivate::Ordering::Equivalent) - return std::partial_ordering::equivalent; - else if (static_cast(m_order) == QtPrivate::Ordering::Greater) - return std::partial_ordering::greater; - else if (static_cast(m_order) == QtPrivate::Uncomparable::Unordered) - return std::partial_ordering::unordered; - return std::partial_ordering::unordered; + switch (m_order) { + case qToUnderlying(QtPrivate::Ordering::Less): return std::partial_ordering::less; + case qToUnderlying(QtPrivate::Ordering::Greater): return std::partial_ordering::greater; + case qToUnderlying(QtPrivate::Ordering::Equivalent): return std::partial_ordering::equivalent; + case qToUnderlying(QtPrivate::Uncomparable::Unordered): return std::partial_ordering::unordered; + } + Q_UNREACHABLE_RETURN(std::partial_ordering::unordered); } friend constexpr bool operator==(partial_ordering lhs, std::partial_ordering rhs) noexcept @@ -416,13 +414,12 @@ public: constexpr Q_IMPLICIT operator std::weak_ordering() const noexcept { - if (static_cast(m_order) == QtPrivate::Ordering::Less) - return std::weak_ordering::less; - else if (static_cast(m_order) == QtPrivate::Ordering::Equivalent) - return std::weak_ordering::equivalent; - else if (static_cast(m_order) == QtPrivate::Ordering::Greater) - return std::weak_ordering::greater; - return std::weak_ordering::equivalent; + switch (m_order) { + case qToUnderlying(QtPrivate::Ordering::Less): return std::weak_ordering::less; + case qToUnderlying(QtPrivate::Ordering::Greater): return std::weak_ordering::greater; + case qToUnderlying(QtPrivate::Ordering::Equivalent): return std::weak_ordering::equivalent; + } + Q_UNREACHABLE_RETURN(std::weak_ordering::equivalent); } friend constexpr bool operator==(weak_ordering lhs, std::weak_ordering rhs) noexcept @@ -596,15 +593,12 @@ public: constexpr Q_IMPLICIT operator std::strong_ordering() const noexcept { - if (static_cast(m_order) == QtPrivate::Ordering::Less) - return std::strong_ordering::less; - else if (static_cast(m_order) == QtPrivate::Ordering::Equivalent) - return std::strong_ordering::equivalent; - else if (static_cast(m_order) == QtPrivate::Ordering::Equal) - return std::strong_ordering::equal; - else if (static_cast(m_order) == QtPrivate::Ordering::Greater) - return std::strong_ordering::greater; - return std::strong_ordering::equivalent; + switch (m_order) { + case qToUnderlying(QtPrivate::Ordering::Less): return std::strong_ordering::less; + case qToUnderlying(QtPrivate::Ordering::Greater): return std::strong_ordering::greater; + case qToUnderlying(QtPrivate::Ordering::Equal): return std::strong_ordering::equal; + } + Q_UNREACHABLE_RETURN(std::strong_ordering::equal); } friend constexpr bool operator==(strong_ordering lhs, std::strong_ordering rhs) noexcept