NEON opts turned on by default

Bug #524859 reported by Loïc Minier on 2010-02-20
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
x264 (Ubuntu)
Medium
Loïc Minier
Lucid
Medium
Loïc Minier

Bug Description

Binary package hint: x264

Hi

NEON opts are turned on by default by the upstream configure; this is incorrect and should be fixed or worked around before release as some armel platforms targetted by lucid don't have NEON.

Bye,

Related branches

Loïc Minier (lool) wrote :

So upstream suggested using --disable-asm for a baseline build, and then using armv6t2 mode with NEON runtime detection, which is the default with a non-NEON fpu allowing v6t2.

Changed in x264 (Ubuntu Lucid):
importance: Undecided → Medium
status: New → Triaged
Loïc Minier (lool) wrote :

NB: In Ubuntu, we probably want a baseline build without --disable-asm (with asm) since v6t2 is implemented as asm.

Loïc Minier (lool) on 2010-02-21
Changed in x264 (Ubuntu Lucid):
assignee: nobody → Loïc Minier (lool)
status: Triaged → Fix Committed
Dave Martin (dave-martin-arm) wrote :

Do we need a baseline build with --disable-asm?

ARMv7 (the Ubuntu target) is a superset of ARMv6T2, so providing that runtime detection is used for features which are not guaranteed to be part of ARMv6T2 and asm code should work on all potential target platforms.

Loïc Minier (lool) wrote :

No we don't, and that's why the changes I did will check for v6t2 and disable an extra optimized build if present in the default toolchain; currently, this should result in only a default build, but turning off -mfpu=neon (it passes -march=armv7-a as detected from gcc -v to disable upstream's -mfpu setting code).

Loïc Minier (lool) wrote :

(On Debian, v6t2 isn't in the default toolchain and that causes a --disable-asm build by default, with an extra NEON build with the upstream default flags)

Dave Martin (dave-martin-arm) wrote :

Patch to avoid building everything with -mfpu=neon, and enable HWCAP-based runtime detection of NEON.

This should give us binaries which work across our ARMv7 targets, and which doesn't use NEON code where inappropriate (Dove / Babbage2/1)

This patch applies against upstream and against lucid x264 (2:0.85.1442.1+git781d30-1).
This patch should work for debian (it should not cause NEON to be enabled where it previously was not).

Upstream might not be so happy about:

  * the --disable-assume-neon configure arg. There might be a better way to enable NEON only in the asm, but this was my best idea at the moment.

  * Disabling of the "fast NEON MRC" test. This performance counter based test may not be safe across all platforms, and won't reliably give the right answer even where it doesn't SIGILL— it also requires a patched kernel; so we probably don't want it enabled in Ubuntu (unless we can use some other decision route like identifying the CPU from the contents of /proc/cpuinfo. I haven't attempted this for now.)

See the next patch for proposed changes to debian/rules.

Dave Martin (dave-martin-arm) wrote :

Patch for debian/rules to pass the new argument --disable-assume-neon to configure for armel

This should apply against Debian without problems, if the previous patch is also applied.

Dave Martin (dave-martin-arm) wrote :

We should try to upstream these (or similar) patches if they look useful.

Reinhard Tartler (siretart) wrote :

what's the status about this bug? What does upstream think about this?

I've now prepared a new upload for x264 and wanted to know if this is something that should go into this upload or can be deferred.

Loïc Minier (lool) wrote :

@Reinhard: this bug will be fixed in your upload, but we need to track another bug which is about not using SIGILL for capabilities detection. For the latter, Dave's patches would need to be adjusted slightly and discussed with upstream, but I didn't come to that. Let's land the debian/rules rework first as to not introduce too many changes in the same upload, if that's ok with you.

Allright. I'll then close this bug. The proposed patch will not be
deleted from launchpad, and we can lookup the bugnumber from the
changelog in any case.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package x264 - 2:0.85.1448+git1a6d32-1

---------------
x264 (2:0.85.1448+git1a6d32-1) lucid; urgency=low

  [ Loic Minier ]
  * Drop --enable-pic, let's see what breaks, LP: #524859

  [ Reinhard Tartler ]
  * New upstream snapshot, no new features, LP: #526396
  * remove quilt infrastructure
  * don't set CFLAGS in debian/rules, upstream build system overrides
    this anyways
 -- Reinhard Tartler <email address hidden> Sun, 21 Feb 2010 16:57:21 +0100

Changed in x264 (Ubuntu Lucid):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers