From cdd0e8fa6772c407f699995d6f1d03434d7f33aa Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 8 May 2023 11:23:31 -0700 Subject: [PATCH] QSlotObjectBase: move the `which` parameter to the 4th position This places the first through third parameters on the exact positions that they will be used to perform the operations in the switch, saving the compiler from generating a few instructions to move data around. All ABIs Qt supports that pass any function parameters in registers at all pass at least 4. We keep the return type as void (instead of returning bool, for the Compare case) so the compiler can apply tail-call optimizations for those two typical cases. PMF case: https://gcc.godbolt.org/z/9oP5boKfj Function case: https://gcc.godbolt.org/z/e9vEzd5dj Functor case: https://gcc.godbolt.org/z/s8Ejjra7P Change-Id: I3e3bfef633af4130a03afffd175d3e3009c56323 Reviewed-by: Fabian Kosmale --- src/corelib/kernel/qobject_p.h | 4 +++ src/corelib/kernel/qobjectdefs_impl.h | 31 ++++++++++++++++--- src/corelib/kernel/qproperty.cpp | 5 +++ src/corelib/kernel/qproperty_p.h | 4 +++ .../corelib/kernel/qobject/tst_qobject.cpp | 4 +++ 5 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index 61a0fe9ebba..de014a5525a 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/src/corelib/kernel/qobject_p.h @@ -268,7 +268,11 @@ template class QPrivateSlotObject : public QSlotObjectBase, private FunctionStorage { typedef QtPrivate::FunctionPointer FuncType; +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) static void impl(int which, QSlotObjectBase *this_, QObject *r, void **a, bool *ret) +#else + static void impl(QSlotObjectBase *this_, QObject *r, void **a, int which, bool *ret) +#endif { const auto that = static_cast(this_); switch (which) { diff --git a/src/corelib/kernel/qobjectdefs_impl.h b/src/corelib/kernel/qobjectdefs_impl.h index 33df3624d22..6f2dac783be 100644 --- a/src/corelib/kernel/qobjectdefs_impl.h +++ b/src/corelib/kernel/qobjectdefs_impl.h @@ -387,17 +387,17 @@ namespace QtPrivate { // internal base class (interface) containing functions required to call a slot managed by a pointer to function. class QSlotObjectBase { -#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) - QAtomicInt m_ref = 1; -#endif // Don't use virtual functions here; we don't want the // compiler to create tons of per-polymorphic-class stuff that // we'll never need. We just use one function pointer, and the // Operations enum below to distinguish requests +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) + QAtomicInt m_ref = 1; typedef void (*ImplFn)(int which, QSlotObjectBase* this_, QObject *receiver, void **args, bool *ret); const ImplFn m_impl; - -#if QT_VERSION >= QT_VERSION_CHECK(7, 0, 0) +#else + using ImplFn = void (*)(QSlotObjectBase* this_, QObject *receiver, void **args, int which, bool *ret); + const ImplFn m_impl; QAtomicInt m_ref = 1; #endif protected: @@ -414,11 +414,24 @@ namespace QtPrivate { explicit QSlotObjectBase(ImplFn fn) : m_impl(fn) {} inline int ref() noexcept { return m_ref.ref(); } +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) inline void destroyIfLastRef() noexcept { if (!m_ref.deref()) m_impl(Destroy, this, nullptr, nullptr, nullptr); } inline bool compare(void **a) { bool ret = false; m_impl(Compare, this, nullptr, a, &ret); return ret; } inline void call(QObject *r, void **a) { m_impl(Call, this, r, a, nullptr); } +#else + inline void destroyIfLastRef() noexcept + { if (!m_ref.deref()) m_impl(this, nullptr, nullptr, Destroy, nullptr); } + + inline bool compare(void **a) + { + bool ret = false; + m_impl(this, nullptr, a, Compare, &ret); + return ret; + } + inline void call(QObject *r, void **a) { m_impl(this, r, a, Call, nullptr); } +#endif bool isImpl(ImplFn f) const { return m_impl == f; } protected: ~QSlotObjectBase() {} @@ -438,7 +451,15 @@ namespace QtPrivate { QtPrivate::FunctionPointer, QtPrivate::Functor >; + +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) Q_DECL_HIDDEN static void impl(int which, QSlotObjectBase *this_, QObject *r, void **a, bool *ret) +#else + // Design note: the first three arguments match those for typical Call + // and Destroy uses. We return void to enable tail call optimization + // for those too. + Q_DECL_HIDDEN static void impl(QSlotObjectBase *this_, QObject *r, void **a, int which, bool *ret) +#endif { const auto that = static_cast(this_); switch (which) { diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index 765a596585c..b6513a0b71d 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -2477,8 +2477,13 @@ QPropertyAdaptorSlotObject::QPropertyAdaptorSlotObject(QObject *o, const QMetaPr { } +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) void QPropertyAdaptorSlotObject::impl(int which, QSlotObjectBase *this_, QObject *r, void **a, bool *ret) +#else +void QPropertyAdaptorSlotObject::impl(QSlotObjectBase *this_, QObject *r, void **a, int which, + bool *ret) +#endif { auto self = static_cast(this_); switch (which) { diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index 9096df25ad4..5b1d0eb2739 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -917,7 +917,11 @@ class QPropertyAdaptorSlotObject : public QUntypedPropertyData, public QSlotObje QObject *obj; QMetaProperty metaProperty_; +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) static void impl(int which, QSlotObjectBase *this_, QObject *r, void **a, bool *ret); +#else + static void impl(QSlotObjectBase *this_, QObject *r, void **a, int which, bool *ret); +#endif QPropertyAdaptorSlotObject(QObject *o, const QMetaProperty& p); diff --git a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp index 16f53d58da3..5cb9faf01f3 100644 --- a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp +++ b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp @@ -6813,7 +6813,11 @@ struct QmlReceiver : public QtPrivate::QSlotObjectBase , magic(0) {} +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) static void impl(int which, QSlotObjectBase *this_, QObject *, void **metaArgs, bool *ret) +#else + static void impl(QSlotObjectBase *this_, QObject *, void **metaArgs, int which, bool *ret) +#endif { switch (which) { case Destroy: delete static_cast(this_); return;