[pulseaudio-discuss] ARM NEON patches

[Rémi Denis-Courmont]

On Fri, 10 Feb 2012 13:59:47 +0100 (CET), Peter Meerwald
<pmeerw at pmeerw.net> wrote:
>> <arun.raghavan at collabora.co.uk> wrote:
>> > The lack of a configuration option is fine. And as I understand it,
the
>> > convention in the ARM world is you compile for a given target and run
>> > only on a machine that is a superset of that target. So, unlike with
>> > MMX/SSE, not having a run-time tests is okay.
> 
>> That's not necessarily true. You can check for NEON at run-time in
>> /proc/cpuinfo, and e.g. libvpx does exactly that. A certain ARM-based
>> Linux
>> kernel-based smartphone operating system has the same ABI for both
>> NEON-capable and NEON-lacking chipsets. I guess Ubuntu is trying to do
>> that
>> too.
> 
> there is no configure option to disable compilation of the NEON code, it

> is however guarded by appropriate #ifdefs
> 
> at runtime the proposed patches already parse /proc/cpuinfo (on Linux at

> least) to figure out if the CPU has the NEON feature

If __ARM_ENON__ is defined, the compiler assumes NEON is available and can
emit NEON opcodes for plain C code. In particular, any code that parses
/proc/cpuinfo for "Features:.*neon" under #ifdef __ARM_NEON__ is completely
*non-sensical*. Yes, this include patch 1 of 4 in your patchset - I already
pointed that out but nobody seemed to pay attention.

It's as if you'd compile PulseAudio with -msse2 and the run the result on
a Pentium I. It might work by luck - if the compiler did not find an
opportunity to use any SSE instruction. But it is not supposed to work.
Similarly, if you pass
"-mfpu=neon", then the compiler assumes the code will run ONLY on
NEON-capable ARM devices. If you want to do run-time detection, you MUST
NOT pass the corresponding compiler flag. (The same is true of MMX and SSE
by the way.)

> the environment variable PULSE_NO_SIMD can be used to disable available 
> optimizations

That won't disable compiler-generated NEON or VFP code, so no.

> I conclude that no action is required w.r.t. configuration

I think you're wrong.

-- 
Rémi Denis-Courmont
http://www.remlab.net/