Qt on armel is built with NEON by default

[Oliver Grawert]

On the armel platform in maveirck we guarantee that no NEON support is hard built in by default into any libs, since we support HW in this release that is not capable of using NEON instructions.

On supported HW like marvell dove (or on the unsupported nvidia tegra2), QT applications all die with SIGILL.

Examining the build scripts (mainly configure) reveals that QT does build time detection for NEON capabilities. Since our buildds are NEON capable (but not all our target architectures) the configure options need to be adjusted to not do the build time NEON detection by default.

For maverick the -no-neon option needs to be handed through to configure from debian/rules. For natty and later releases runtime detection based on hwcaps needs to be implemented in QT.

[Oliver Grawert (ogra)]

a fix for this issue was uploaded to maverick-poposed, SRU team, please approve

[Oliver Grawert (ogra)]

qt4-x11 (4:4.7.0-0ubuntu4.1) maverick-proposed; urgency=low

  * disable NEON on armel builds (LP: #664431)

 -- Oliver Grawert <email address hidden> Thu, 21 Oct 2010 14:45:54 +0200

[Martin Pitt (pitti)]

Accepted qt4-x11 into maverick-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Rodrigo Belem (rbelem)]

Hello,

There is already a way to disable NEON on runtime as you can see in the following commit in qt.
http://qt.gitorious.org/qt/qt/commit/ebf9d5dd5174b7c82fec83b56d71a59d5277bd51

export QT_NO_CPU_FEATURE=neon

How could we properly use this feature?

[Ricardo Salveti (rsalveti)]

Is it possible to enable it at runtime? Because by default we should avoid using NEON.

[Ricardo Salveti (rsalveti)]

And the best option for us would be automatic detection at runtime, so we don't need to use any env variable. It seems Linaro is working on that, but don't know if that's ready already.

[Oliver Grawert (ogra)]

http://www.llvm.org/bugs/attachment.cgi?id=4428 is an example how to do runtime NEON detection through hwcaps (using /proc/self/auxv)

[Scott Kitterman (kitterman)]

This would be a post release regression for hardware that has NEON support. Is this really appropriate for an SRU?

[Steve Langasek (vorlon)]

it is a performance regression on some hardware, in exchange for correcting a regression in basic functionality of the package on other supported hardware. In the absence of other solutions, I think this is preferable over the status quo. Do you disagree?

[Ricardo Salveti (rsalveti)]

Talking with Thiago Macieira at UDS he said that Qt already has auto detection for NEON at upstream. Maybe we can identify the proper commit and just backport the fix, then we'll have the best of both worlds.

[Scott Kitterman (kitterman)]

My understanding is it's fixed in 4.7.1 so it should be manageable for someone to dig the proper fix out of Qt got.

[Scott Kitterman (kitterman)]

"Steve Langasek" <email address hidden> wrote:

>it is a performance regression on some hardware, in exchange for
>correcting a regression in basic functionality of the package on other
>supported hardware. In the absence of other solutions, I think this is
>preferable over the status quo. Do you disagree?

I believe the foundational assumption that there is a lack of alternative options is premature at best. The lack of runtime detection is a bug that upstream says is fixed in Qt 4.7.1.

In general I think that prior to uploading an SRU, and in particular before uploading an SRU like this that guarantees regression for some users, it would be useful to have a conversation with the Ubuntu team that does most of the maintenance work on a package.

[Matthias Klose (doko)]

On 27.10.2010 01:58, Scott Kitterman wrote:
>
> "Steve Langasek"<email address hidden> wrote:
>
>> it is a performance regression on some hardware, in exchange for
>> correcting a regression in basic functionality of the package on other
>> supported hardware. In the absence of other solutions, I think this is
>> preferable over the status quo. Do you disagree?
>
> I believe the foundational assumption that there is a lack of
> alternative options is premature at best. The lack of runtime detection
> is a bug that upstream says is fixed in Qt 4.7.1.
>
> In general I think that prior to uploading an SRU, and in particular
> before uploading an SRU like this that guarantees regression for some
> users, it would be useful to have a conversation with the Ubuntu team
> that does most of the maintenance work on a package.

I don't get it. I certainly prefer a working package on all supported
platforms, even if it runs slower on some of them. We would do the same on
other platforms if we would detect a package built with, e.g. -msse2

   Matthias

[Martin Pitt (pitti)]

Matthias Klose [2010-10-27 14:44 -0000]:
> I don't get it. I certainly prefer a working package on all supported
> platforms, even if it runs slower on some of them. We would do the same on
> other platforms if we would detect a package built with, e.g. -msse2

I agree. We specifically said that we don't want NEON enabled in any
package (yet), so this has never been intended.

[Ricardo Salveti (rsalveti)]

On Wed, Oct 27, 2010 at 11:42 AM, Martin Pitt <email address hidden> wrote:
> Matthias Klose [2010-10-27 14:44 -0000]:
>> I don't get it.  I certainly prefer a working package on all supported
>> platforms, even if it runs slower on some of them.  We would do the same on
>> other platforms if we would detect a package built with, e.g. -msse2
>
> I agree. We specifically said that we don't want NEON enabled in any
> package (yet), so this has never been intended.

Sure, but as it does improve the performance a lot on hardwares that
support NEON, I also agree that this is post release regression.

For users for sure it would sound weird to have a fix that actually
decreases a lot the performance of qt based applications on their
hardware after an update.

[Rodrigo Belem (rbelem)]

Let's just backport the patch that adds neon auto detection support.

[Oliver Grawert (ogra)]

could someone point to the matching commit so we can see the impact ?

i dont think we actually care in which way neon is disabled on platforms that dont support it as long as you dont get a SIGILL for all QT apps you try to start.
platforms that support NEON will indeed benefit from it but we made a commitment that nothing in the archive is built with NEON for maverick.

it is not acceptable to build QT with NEON statically enabled at build time, i think its a good compromise to use runtime detection if it:
a) gets enough testing so we are confident it doesnt add additional regression
and
b) the patch is small enough

[Martin Pitt (pitti)]

Marking as v-failed for now. Keeping in -proposed is fine, since the performance regression isn't really breaking anything. From the recent comments it seems it should be possible to fix the runtime detection, so I agree that this would be the better approach.

[Thiago Macieira (thiago-kde)]

This is the actual commit that introduces runtime verification of processor features on ARM:
 http://qt.gitorious.org/qt/qt/commit/5070c3ae331faf18f6997535356853cc61ef0ad7

It was introduced before Qt 4.7.0rc1 so the Ubuntu build should have it.

Therefore, there are two possibilities:
1) the detection mechanism is faulty and concluding that neon is present when it isn't
2) the detection mechanism is working but there's some unprotected Neon somewhere

To check #1, please take a Qt built without "-no-neon", place it on a device without Neon and then run libQtCore.so.4. The last line of the output should be "Processor features:" and list probably nothing.

If it is case #2, then can someone post a backtrace of the SIGILL?

[Ricardo Salveti (rsalveti)]

On Thu, Oct 28, 2010 at 9:09 AM, Thiago Macieira <email address hidden> wrote:
> This is the actual commit that introduces runtime verification of processor features on ARM:
>  http://qt.gitorious.org/qt/qt/commit/5070c3ae331faf18f6997535356853cc61ef0ad7
>
> It was introduced before Qt 4.7.0rc1 so the Ubuntu build should have it.
>
> Therefore, there are two possibilities:
> 1) the detection mechanism is faulty and concluding that neon is present when it isn't
> 2) the detection mechanism is working but there's some unprotected Neon somewhere
>
> To check #1, please take a Qt built without "-no-neon", place it on a
> device without Neon and then run libQtCore.so.4. The last line of the
> output should be "Processor features:" and list probably nothing.
>
> If it is case #2, then can someone post a backtrace of the SIGILL?

I currently can't get the git code (download at 8k/s), so I checked
the 4.7.0 release tarball and this commit doesn't seems to be
included:
"""
$ vim qt-everywhere-opensource-src-4.7.0/src/corelib/tools/qsimd.cpp
---
#elif defined(QT_HAVE_IWMMXT)
    // runtime detection only available when running as a previlegied process
    static const bool doIWMMXT = !qgetenv("QT_NO_IWMMXT").toInt();
    features = doIWMMXT ? IWMMXT : 0;
    return features;
#elif defined(QT_HAVE_NEON)
    static const bool doNEON = !qgetenv("QT_NO_NEON").toInt();
    features = doNEON ? NEON : 0;
    return features;
"""

Will backport it and create a proper package to be tested, once I can
download the sources.

[Thiago Macieira (thiago-macieira)]

On Thursday, 28 de October de 2010 07:55:34 you wrote:
> I currently can't get the git code (download at 8k/s), so I checked
> the 4.7.0 release tarball and this commit doesn't seems to be
> included:
> """
> $ vim qt-everywhere-opensource-src-4.7.0/src/corelib/tools/qsimd.cpp
> ---
> #elif defined(QT_HAVE_IWMMXT)
> // runtime detection only available when running as a previlegied
> process static const bool doIWMMXT = !qgetenv("QT_NO_IWMMXT").toInt();
> features = doIWMMXT ? IWMMXT : 0;
> return features;
> #elif defined(QT_HAVE_NEON)
> static const bool doNEON = !qgetenv("QT_NO_NEON").toInt();
> features = doNEON ? NEON : 0;
> return features;
> """
>
> Will backport it and create a proper package to be tested, once I can
> download the sources.

Oh, that might explain it all. The fix wasn't cherry-picked into the 4.7.0
release.

Just take the two qsimd files and it should be your patch:

 http://qt.gitorious.org/qt/qt/blobs/4.7/src/corelib/tools/qsimd.cpp
 http://qt.gitorious.org/qt/qt/blobs/4.7/src/corelib/tools/qsimd_p.h

Patch attached on top of 4.7.0.

--
Thiago Macieira - thiago.macieira (AT) nokia.com
  Senior Product Manager - Nokia, Qt Development Frameworks
     Sandakerveien 116, NO-0402 Oslo, Norway

[Oliver Grawert (ogra)]

if i read that code right it enforces NEON use if it cant open /proc/self/auxv ...
sorry, but thats not acceptable, it should use a conservative default instead, can that be changed ?

otherwise the patch looks fine

[Thiago Macieira (thiago-macieira)]

On Friday, 29 de October de 2010 07:22:28 you wrote:
> if i read that code right it enforces NEON use if it cant open
> /proc/self/auxv ... sorry, but thats not acceptable, it should use a
> conservative default instead, can that be changed ?

Yes, I think it's a good idea.
--
Thiago Macieira - thiago.macieira (AT) nokia.com
  Senior Product Manager - Nokia, Qt Development Frameworks
     Sandakerveien 116, NO-0402 Oslo, Norway

[Ricardo Salveti (rsalveti)]

I ported the following patch to have NEON autodetection. It's basically the same logic as upstream, but without putting NEON as default in case it cant find /proc/self/auxv. It also allows the user to run without NEON, by exporting QT_NO_NEON.

I'm currently trying to build the Qt package with this patch, but my main panda seems dead now and I don' t have physical access to reboot it until wednesday. Will try to compile with another panda that's online, but using mmc as the rootfs, so it'll take some long hours.

[Dave Martin (dave-martin-arm)]

@Richard,

Are QT_HAVE_IWMMXT and QT_HAVE_NEON mutually exclusive?

Currently, if QT_HAVE_IWMMXT _or_ QT_HAVE_NEON is set to 1, this causes both features to be suppressed, which may not be what is desired if both extensions can be enabled in a general-purpose bulid.

Maybe something like this:

int features = 0;
int features_mask = 0;

#ifdef QT_HAVE_IWMMXT
if(qgetenv("QT_NO_IWMXXT").toInt() != 0)
    features_mask |= HWCAP_NEON;
#endif

#ifdef QT_HAVE_NEON
if(qgetenv("QT_NO_NEON").toInt() != 0)
    features_mask |= HWCAP_NEON;
#endif

#if defined(Q_OS_LINUX)
/* features = <result of querying /proc/self/auxv> */
#endif

return features & ~features_mask;

[Ricardo Salveti (rsalveti)]

@Dave: this was basically the previous behavior, that I didn't want to change because it's outside the scope of this bug.

From current Qt in Maverick:
...
#elif defined(QT_HAVE_IWMMXT)
    // runtime detection only available when running as a previlegied process
    static const bool doIWMMXT = !qgetenv("QT_NO_IWMMXT").toInt();
    features = doIWMMXT ? IWMMXT : 0;
    return features;
#elif defined(QT_HAVE_NEON)
    static const bool doNEON = !qgetenv("QT_NO_NEON").toInt();
    features = doNEON ? NEON : 0;
    return features;
#else
    features = 0;
...

Upstream doesn't support the QT_NO_* flags anymore, but when probing the default values you can see:
...
#if defined(QT_HAVE_IWMMXT)
    // runtime detection only available when running as a previlegied process
    features = IWMMXT;
#elif defined(QT_HAVE_NEON)
    features = NEON;
#endif
...

While porting the fix we can't just drop the QT_NO_* flags, so I just let it as the previous code, plus adding neon autodetection code path.

[Ricardo Salveti (rsalveti)]

Was finally able to build a new qt package with the attached patch. Oliver, in case you want to test please get the packages from our private PPA, or wait the packages to be copied to my own public PPA at https://launchpad.net/~rsalveti/+archive/armel.

Please test it at your AC100 and let us know if it worked or not for you.

[Steve Langasek (vorlon)]

Removed qt4-x11 4:4.7.0-0ubuntu4.1 from maverick-proposed due to the regression. Please upload this fixed version with runtime detection support when you have a chance.

[Oliver Grawert (ogra)]

testing the packages from ricardos PPA on a non NEON architecture results in a SIGILL again :(

[Thiago Macieira (thiago-macieira)]

Em Segunda-feira 08 Novembro 2010, às 16:50:52, você escreveu:
> testing the packages from ricardos PPA on a non NEON architecture
> results in a SIGILL again :(

Can you post a backtrace? It would be useful if the build used -fno-inline, or
this will get a bit tricky to track down.

Please also verify that Qt did detect that Neon is absent. Run libQtCore.so.4
and paste the last line of the output. E.g.:

# /usr/lib/libQtCore.so.4
This is the QtCore library version 4.7.1
Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
Contact: Nokia Corporation (<email address hidden>)

Build key: armv6 linux g++-4 full-config
Compat build key: |
Build date: 2010-11-04
Installation prefix: /usr
Library path: /usr/lib
Include path: /usr/include/qt4
Processor features: neon

If it says "neon" is present (like above), then it looks like there was a
detection mistake. Can you try turning the feature off by setting
QT_NO_CPU_FEATURE=neon and running the app again?

Finally, it would be nice also to have the command-line of the compiler used
when compiling the actual .cpp file.

--
Thiago Macieira - thiago.macieira (AT) nokia.com
  Senior Product Manager - Nokia, Qt Development Frameworks
     Sandakerveien 116, NO-0402 Oslo, Norway

[Oliver Grawert (ogra)]

root@ac100:~# /usr/lib/libQtCore.so.4.7.0
This is the QtCore library version 4.7.0
Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
Contact: Nokia Corporation (<email address hidden>)

Build key: armv6 linux g++-4 full-config
Build date: 2010-11-04
Installation prefix: /usr
Library path: /usr/lib
Include path: /usr/include/qt4
root@ac100:~# exit
ogra@ac100:~$ QT_NO_CPU_FEATURE=neon mumble
Illegal instruction

a backtrace will follow soon ...

[Ricardo Salveti (rsalveti)]

On Mon, Nov 8, 2010 at 2:09 PM, Thiago Macieira
<email address hidden> wrote:
> Em Segunda-feira 08 Novembro 2010, às 16:50:52, você escreveu:
>> testing the packages from ricardos PPA on a non NEON architecture
>> results in a SIGILL again :(
>
> Can you post a backtrace? It would be useful if the build used -fno-inline, or
> this will get a bit tricky to track down.
>
> Please also verify that Qt did detect that Neon is absent. Run libQtCore.so.4
> and paste the last line of the output. E.g.:
>
> # /usr/lib/libQtCore.so.4
> This is the QtCore library version 4.7.1
> Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
> Contact: Nokia Corporation (<email address hidden>)
>
> Build key:           armv6 linux g++-4 full-config
> Compat build key:    |
> Build date:          2010-11-04
> Installation prefix: /usr
> Library path:        /usr/lib
> Include path:        /usr/include/qt4
> Processor features:  neon
>
> If it says "neon" is present (like above), then it looks like there was a
> detection mistake. Can you try turning the feature off by setting
> QT_NO_CPU_FEATURE=neon and running the app again?

4.7, the one used by Maverick doesn't export the processor features line.

Best thing would be to get the proper trace from the sigill.

> Finally, it would be nice also to have the command-line of the compiler used
> when compiling the actual .cpp file.

One example:
g++ -c -include .pch/release-shared/QtCore -g -O2
-I/usr/include/freetype2 -pthread -I/usr/include/glib-2.0
-I/usr/lib/glib-2.0/include -mfpu=neon -O2 -fvisibility=hidden
-fvisibility-inlines-hidden -Wall -W -D_REENTRANT -fPIC -DQT_SHARED
-DQT_BUILD_CORE_LIB -DQT_NO_USING_NAMESPACE -DQT_NO_CAST_TO_ASCII
-DQT_ASCII_CAST_WARNINGS -DQT3_SUPPORT -DQT_MOC_COMPAT
-DQT_USE_FAST_OPERATOR_PLUS -DQT_USE_FAST_CONCATENATION
-DELF_INTERPRETER=\"/lib/ld-linux.so.3\" -DHB_EXPORT=Q_CORE_EXPORT
-DQT_HAVE_NEON -DQT_NO_DEBUG -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE
-I../../mkspecs/linux-g++ -I. -I../../include -I../../include/QtCore
-I.rcc/release-shared -Iglobal -I../3rdparty/harfbuzz/src
-I../3rdparty/md5 -I../3rdparty/md4 -I.moc/release-shared -o
.obj/release-shared/qsimd.o tools/qsimd.cpp

[Thiago Macieira (thiago-macieira)]

Em Segunda-feira 08 Novembro 2010, às 19:40:34, você escreveu:
> One example:
> g++ -c -include .pch/release-shared/QtCore -g -O2
> -I/usr/include/freetype2 -pthread -I/usr/include/glib-2.0
> -I/usr/lib/glib-2.0/include -mfpu=neon -O2 -fvisibility=hidden
> -fvisibility-inlines-hidden -Wall -W -D_REENTRANT -fPIC -DQT_SHARED
> -DQT_BUILD_CORE_LIB -DQT_NO_USING_NAMESPACE -DQT_NO_CAST_TO_ASCII
> -DQT_ASCII_CAST_WARNINGS -DQT3_SUPPORT -DQT_MOC_COMPAT
> -DQT_USE_FAST_OPERATOR_PLUS -DQT_USE_FAST_CONCATENATION
> -DELF_INTERPRETER=\"/lib/ld-linux.so.3\" -DHB_EXPORT=Q_CORE_EXPORT
> -DQT_HAVE_NEON -DQT_NO_DEBUG -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE
> -I../../mkspecs/linux-g++ -I. -I../../include -I../../include/QtCore
> -I.rcc/release-shared -Iglobal -I../3rdparty/harfbuzz/src
> -I../3rdparty/md5 -I../3rdparty/md4 -I.moc/release-shared -o
> .obj/release-shared/qsimd.o tools/qsimd.cpp

See the -mfpu=neon there.

I think I see the problem. It's upstream.
--
Thiago Macieira - thiago.macieira (AT) nokia.com
  Senior Product Manager - Nokia, Qt Development Frameworks
     Sandakerveien 116, NO-0402 Oslo, Norway

[Thiago Macieira (thiago-macieira)]

Em Terça-feira 09 Novembro 2010, às 10:27:16, você escreveu:
> See the -mfpu=neon there.
>
> I think I see the problem. It's upstream.

Upstream report created: http://bugreports.qt.nokia.com/browse/QTBUG-15150

--
Thiago Macieira - thiago.macieira (AT) nokia.com
  Senior Product Manager - Nokia, Qt Development Frameworks
     Sandakerveien 116, NO-0402 Oslo, Norway

[Oliver Grawert (ogra)]

given that there is not much movement yet on the upstream bug, i re-uploaded 4.7.0-0ubuntu4.1 that disables NEON statically. so people without NEON finally can use QT apps. once the upstream fix is found and implemented i'm happy to revert the change again for dynamic runtime NEON detection.

SRU team, please accept into maverick-proposed

[Oliver Grawert (ogra)]

i bumped the version to ubuntu4.2 since there is a 4.1 source in the archive already, sorry for the inconvenience

[Ricardo Salveti (rsalveti)]

I'll make a public PPA available with the neon compatible Qt, so people can continue using the released version with neon accelerated pieces.

[Martin Pitt (pitti)]

Accepted qt4-x11 into maverick-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Ricardo Salveti (rsalveti)]

On Fri, Nov 19, 2010 at 2:44 PM, Ricardo Salveti <email address hidden> wrote:
> I'll make a public PPA available with the neon compatible Qt, so people
> can continue using the released version with neon accelerated pieces.

Qt PPA with NEON enabled: https://launchpad.net/~rsalveti/+archive/qt-neon

Please test and let me know if it works as expected.

[Oliver Grawert (ogra)]

installing mumble (QT app) on a NEON-less tegra2 system with the 4.7.0-0ubuntu4.2 QT packages from maverick-proposed works fine, no SIGILL, app starts and is functional

[Martin Pitt (pitti)]

Please get this fixed in natty (one way or the other), so that the fix can proceed to maverick-updates.

[Alexander Sack (asac)]

guys, couldnt you try to fix the build system to not pass -mfpu=neon if neon is detected, but still include arm_neon.h ... which i assume will enable the manually coded neon paths depending on whether the hw feature is on the host?

if that wasnt tried, please consider to do that before regressing all the NEON enabled boards. note that there is not even a single board without NEON supported officially. So while this should be fixed there shouldn't be a reason to rush things here imo.

[Alexander Sack (asac)]

subscribed a few folks from linaro graphics WG ... maybe they can help here if they have some cycles.

[Jamie Bennett (jamiebennett)]

Agreed.

Our official enablement this cycle is for NEON _enabled_ platforms only. Whilst supporting non-NEON boards is an eventual goal it should not be at the detriment of the platforms we have committed to for Natty.

[Oliver Grawert (ogra)]

this bug is about a maverick SRU where ubuntu fully supports non NEON architectures, we will upload the static fix to natty too during this week until the upstream fixes are done. given there is no movement upstream yet and the SRU team expects the fix to be in natty as well for accepting it into -updates, this has to happen now ...

as i mentioned above, if proper runtime detection is in the upstream source it is fine to switch to it. it is not acceptable to let maverick users wait any longer given that the fix is now available since over a month and was already reverted falsely once.

[Scott Kitterman (kitterman)]

This is going to hurt users of neon capable systems (which is almost everyone). I don't think this SRU is consistent with Ubuntu policy.

[David Mandala (davidm)]

The archive is supposed to support the entire ARMv7 class of devices, and in that class of devices NEON was/is optional we can only have run time support for NEON, not statically hard compiled in. What devices we directly support can change even now, so any statically hard compiled in NEON support is a bug.

[Martin Pitt (pitti)]

Scott Kitterman [2010-11-30 13:15 -0000]:
> I don't think this SRU is consistent with Ubuntu policy.

I accepted it under the premise that it is a regression from lucid
(read: all Qt applications crash right away with a SIGILL). It
happens on a dove board I have laying around, but I don't know how
"typical" this thing is.

[David Mandala (davidm)]

The Dove board is a supported platform and is one of the ARMv7 class platforms that does not support NEON. Martin you have made the correct choice, it is indeed a "regression" from lucid. All NEON support in the archive must be run time detected to make sure the CPU in question can actually support NEON.

[Loïc Minier (lool)]

I support entirely David's position: we committed to delivering an userspace not requiring NEON, independently from the binary kernels or the images directly in Ubuntu. This is true of both maverick and lucid.

The fact that the whole of Qt was built with NEON is a bug, and there is no doubt that we should fix this hard requirement of NEON in an important library.

NEON will still be used at runtime, so not every optimization is lost.

There are multiple ways to deal with the speed regression, in a second step:
a) we could arrange for an armel PPA with a Qt rebuilt with NEON (this is true of any package BTW); this requires special permission, and ongoing maintenance, but Qt isn't updated frequently in stable releases.
b) we could provide a new qt4-x11-neon source package + alternate binaries in maverick-updates
c) we could change qt4-x11 to build two sets of libraries, albeit this would be considerably heavier in build time

I think b) is not too intrusive if we drop the libs in lib/vfp/neon, we could even pull them in installed system via a Recommends from qt4-x11, albeit I do find this part intrusive, much more than a speed difference on ARM systems.

[Oliver Grawert (ogra)]

> There are multiple ways to deal with the speed regression, in a second step:
> a) we could arrange for an armel PPA with a Qt rebuilt with NEON (this is true of any package BTW); this requires special permission, and ongoing maintenance, but Qt isn't updated frequently in stable releases.
> b) we could provide a new qt4-x11-neon source package + alternate binaries in maverick-updates
> c) we could change qt4-x11 to build two sets of libraries, albeit this would be considerably heavier in build time

a) is already implemented for maverick, rsalveti, can you make sure to
also provide natty builds in the PPA ? so we can finally close this bug
until upstream fixed their build system and tested runtime detection.

[Ricardo Salveti (rsalveti)]

On Tue, Nov 30, 2010 at 11:58 AM, Oliver Grawert <email address hidden> wrote:
>> There are multiple ways to deal with the speed regression, in a second step:
>> a) we could arrange for an armel PPA with a Qt rebuilt with NEON (this is true of any package BTW); this requires special permission, and ongoing maintenance, but Qt isn't updated frequently in stable releases.
>> b) we could provide a new qt4-x11-neon source package + alternate binaries in maverick-updates
>> c) we could change qt4-x11 to build two sets of libraries, albeit this would be considerably heavier in build time
>
> a) is already implemented for maverick, rsalveti, can you make sure to
> also provide natty builds in the PPA ? so we can finally close this bug
> until upstream fixed their build system and tested runtime detection.

Sure, will post it back when it's available at the PPA.

[Ricardo Salveti (rsalveti)]

On Tue, Nov 30, 2010 at 11:15 AM, Scott Kitterman <email address hidden> wrote:
> This is going to hurt users of neon capable systems (which is almost
> everyone). I don't think this SRU is consistent with Ubuntu policy.

It is going to hurt users of neon capable systems, and it's a post
release regression, that's why we first tried to fix the runtime
detection to be able to have the proper fix for both cases.

Problem is that we found that this bug is upstream, and still not
fixed. As we said publicly that our archive is fully compatible with
no-NEON devices, and the bug is still to be fixed upstream, we think
that the best and easier option for now would be to revert it at
archive (with Oliver's upload) and put a NEON capable version at the
PPA.

[Scott Kitterman (kitterman)]

I understand it's a regression for some hardware from Lucid. This SRU is a
regression for a different set of hardware. That's mitigated by the PPA
packages that are being provided, but I think it's a reasonably novel
interpretation of our SRU policy to tell users that had reasonably working
hardware at release that they now need to use a PPA to avoid a regression.

[Thiago Macieira (thiago-kde)]

My recommendation (as upstream) is that you compile Qt with run-time-detected Neon support. That is, do not enable -mfpu=neon everywhere, but let Qt detect that the compiler supports this option and enable only for the *_neon.cpp files.

I knew that Qt had this supported at UDS, but what I didn't know was that it was broken. It wasn't runtime detection, it simply turned the option on for everything. Hence the SIGILL.

I opened a bug report and it was fixed: http://bugreports.qt.nokia.com/browse/QTBUG-15163

There's still something wrong in the detection machinery (task QTBUG-15150) which AFAIK doesn't affect the Ubuntu builds. The issue I know of is a false negative of Neon: it disables Neon even when the compiler supports. That's the MeeGo bug linked from QTBUG-15150.

[Launchpad Janitor (janitor)]

This bug was fixed in the package qt4-x11 - 4:4.7.0-0ubuntu4.2

---------------
qt4-x11 (4:4.7.0-0ubuntu4.2) maverick-proposed; urgency=low

  * disable NEON on armel builds (LP: #664431)
 -- Oliver Grawert <email address hidden> Thu, 21 Oct 2010 14:45:54 +0200

[Oliver Grawert (ogra)]

after a discussion on IRC we decided that we will not apply the static non-NEON build to natty for now to give upstream the opportunity for a proper fix of the runtime detection.

If this fix is not available by natty beta either option a) or option b) from comment #50 above will be implemented.

[Ricardo Salveti (rsalveti)]

Ok, for Natty no PPA will be provided for now as it'll just FTBFS anyway. The idea for Natty is to get the proper fix from upstream and be able to do runtime detection.

[Oliver Grawert (ogra)]

Am Dienstag, den 30.11.2010, 14:24 +0000 schrieb Scott Kitterman:
> I understand it's a regression for some hardware from Lucid. This SRU is a
> regression for a different set of hardware. That's mitigated by the PPA
> packages that are being provided, but I think it's a reasonably novel
> interpretation of our SRU policy to tell users that had reasonably working
> hardware at release that they now need to use a PPA to avoid a regression.
>
the problem is that this package broke policy (no static NEON builds in
the archive) from the beginning and the policy breakage went unnoticed,
it should have never entered the archive in this state.

either proper runtime detection has to be in place or one of the
solutions from comment #50 need to be implemented on an archive level.

[Jammy Zhou (jammy-zhou)]

I think we need to centralize all NEON asm code in one or several files, and build these source files with "-mfpu=neon" option, but for other files, "-mfpu=neon" option should not be used when compiling.

[Thiago Macieira (thiago-kde)]

That's already so:

$ git ls-files *_neon*
src/gui/image/qimage_neon.cpp
src/gui/painting/qdrawhelper_neon.cpp
src/gui/painting/qdrawhelper_neon_asm.S
src/gui/painting/qdrawhelper_neon_p.h

These files are the ones that contain 99% of the Neon code in Qt and they're properly run-time detected now (as far as I know).

The only unprotected code is in qstring.cpp: the cost of runtime detection is higher than the benefit, so we decided not to do it. If you compile Qt for a processor with Neon, you get it, otherwise you don't.

[Alain Kalker (miki4242)]

@Thiago

Huh? I don't get it. I haven't looked at the code (bugdiffs is down atm), but don't you need to do the run-time detection only once? Or is a simple compare and branch too much of a perfirmance hit?
Just curious.

[Thiago Macieira (thiago-kde)]

Sorry, let me be more specific:

1) there's a problem with the configure-time code that detects whether the compiler supports Neon intrinsics. Somehow, MeeGo managed to get a false positive here and configure decides that the compiler doesn't support Neon. This is not fixed.

2) there was a problem with the configure-time code that turned on the Neon intrinsics support. It did not support run-time detection: instead, if it figured that the compiler supported Neon intrinsics, it turned Neon everywhere by adding the "-mfpu=neon" option. At the same time, the code with Neon intrinsics was statically enabled, bypassing the runtime detection. This has been fixed.

3) qstring.cpp contains the only code in Qt that uses Neon intrinsics but does not check at runtime. It is enabled at compile-time if the compiler was launched with -mfpu=neon. There are two reasons for this:
 a) the complexity involved in supporting run-time detection for code such as qstring.cpp, as it requires creating a separate .cpp file, compiling it with different compiler options, etc.
 b) there's an associated runtime overhead with the checking of the variable and making the function call to the Neon-enabled code (which cannot be inlined, as it is in a separate .cpp)
So we concluded that if you want to build Qt without Neon, you may as well live without the Neon optimisations for strings.

[Jammy Zhou (jammy-zhou)]

In this case, can we disable the NEON optimization code in qstring.cpp temporarily? I checked latest qstring.cpp, and there is only NEON for QString::fromLatin1_helper() function.

[Thiago Macieira (thiago-kde)]

Like I explained, the code is enabled if and only if qstring.cpp is compiled with -mfpu=neon. We don't turn that on anymore, ever.

If you want that option, you need to set the configure flags yourself.

[Jani Monoses (jani)]

The upstream bug is marked as fixed and part of the upcoming 4.7.2 release. Hopefully that will fix this for natty.

[Jani Monoses (jani)]

Unpatched Qt 4.7.2 FTBFS as -mfpu=neon is not passed to two cpp files (painting/drawhelper_neon.cpp and image/qimage_neon.cpp) .

This fixes it but it needs better variable naming probably, I just went for minimal number of lines changed.

Index: qt4-x11-4.7.2/src/gui/gui.pro
===================================================================
--- qt4-x11-4.7.2.orig/src/gui/gui.pro 2011-03-06 21:41:47.058276259 +0200
+++ qt4-x11-4.7.2/src/gui/gui.pro 2011-03-06 21:38:16.569232507 +0200
@@ -65,9 +65,9 @@
 neon:*-g++* {
     DEFINES += QT_HAVE_NEON
     HEADERS += $$NEON_HEADERS
- SOURCES += $$NEON_SOURCES

     DRAWHELPER_NEON_ASM_FILES = $$NEON_ASM
+ DRAWHELPER_NEON_ASM_FILES += $$NEON_SOURCES

     neon_compiler.commands = $$QMAKE_CXX -c -mfpu=neon
     neon_compiler.commands += $(CXXFLAGS) $(INCPATH) ${QMAKE_FILE_IN} -o ${QMAKE_FILE_OUT}

[Jani Monoses (jani)]

The package has built, it needs testing on non-NEON hw.

[Oliver Grawert (ogra)]

just to confirm, works fine on the ac100 (tegra2 no neon) using unity-2d here in natty

[Phoenix Revived (phoenixrevived)]

I am having the same problem on the Lucid version of armel with Qt 4.6.2
Is a backport planned for this since we can't upgrade to Qt 4.7 which breaks several applications?

[Scott Kitterman (kitterman)]

No backport is contemplated. Backporting Qt breaks too many applications.

[Oliver Grawert (ogra)]

this is an ubuntu build time issue, doesnt affect upstream Qt, closing this task as invalid