[pulseaudio-discuss] [PATCH 01/13] cpu: Add force_generic_code flag to cpu_info struct

[Peter Meerwald]

> > > > The remapper and channel mixing code have (faster) specialized and
> > > > (slower)
> > > > generic code certain code path. The flag force_generic_code can be set
> > > > to
> > > > force the generic code path which is useful for testing. Code
> > > > duplication
> > > > (such as in mix-special-test) can be avoided, cleanup patches follow.
> > > > 
> > > > Signed-off-by: Peter Meerwald <pmeerw at pmeerw.net>
> > > > ---
> > > >    src/Makefile.am       |    2 +-
> > > >    src/daemon/main.c     |    4 +++-
> > > >    src/pulsecore/cpu.c   |   28 ++++++++++++++++++++++++++++
> > > >    src/pulsecore/cpu.h   |    5 +++++
> > > >    src/pulsecore/mix.c   |    8 ++++++++
> > > >    src/pulsecore/remap.c |   13 +++++++++++++
> > > >    6 files changed, 58 insertions(+), 2 deletions(-)
> > > >    create mode 100644 src/pulsecore/cpu.c
> > > > 
> > > > diff --git a/src/Makefile.am b/src/Makefile.am
> > > > index 51ef690..634e26b 100644
> > > > --- a/src/Makefile.am
> > > > +++ b/src/Makefile.am
> > > > @@ -897,7 +897,7 @@ libpulsecore_ at PA_MAJORMINOR@_la_SOURCES = \
> > > >    		pulsecore/rtpoll.c pulsecore/rtpoll.h \
> > > >    		pulsecore/stream-util.c pulsecore/stream-util.h \
> > > >    		pulsecore/mix.c pulsecore/mix.h \
> > > > -		pulsecore/cpu.h \
> > > > +		pulsecore/cpu.c pulsecore/cpu.h \
> > > >    		pulsecore/cpu-arm.c pulsecore/cpu-arm.h \
> > > >    		pulsecore/cpu-x86.c pulsecore/cpu-x86.h \
> > > >    		pulsecore/cpu-orc.c pulsecore/cpu-orc.h \
> > > > diff --git a/src/daemon/main.c b/src/daemon/main.c
> > > > index 02a8ea6..74527be 100644
> > > > --- a/src/daemon/main.c
> > > > +++ b/src/daemon/main.c
> > > > @@ -1050,13 +1050,15 @@ int main(int argc, char *argv[]) {
> > > >    #endif
> > > > 
> > > >        c->cpu_info.cpu_type = PA_CPU_UNDEFINED;
> > > > +    c->cpu_info.force_generic_code = false; /* use generic, slow code
> > > > */
> > > 
> > > Are you sure this comment is correct? It looks like the opposite.
> > 
> > the comment is not very clear at least; it describes the purpose of the
> > flag, not the assignment
> > 
> > replacing it with
> > /* don't force generic code, used for testing only */
> > in v2
> > 
> > > Also, what is the difference between setting force_generic_code and
> > > setting
> > > PULSE_NO_SIMD?
> > 
> > mixing and remapping as generic and special-case code; the flag can be
> > set to force use of the generic code
> > this is useful for the test code so that special-case code can be compared
> > with the (presumably correct) generic code -- it saves quite a lot of code
> > in the test suits (the alternative would be to expose the internal
> > mixing/remapping function); special-case code path are beneficial on all
> > CPUs
> > 
> > PULSE_NO_SIMD suppresses CPU-specific code path
> 
> I first confused "special-case code" with "CPU-specific code", but is this
> correct:
> "special-case code" is when you e g have a special function for "mono to
> stereo" conversion instead of the generic "m to n channels" conversion,
> whereas "CPU-specific code" means code that uses special instructions (usually
> hand written assembly)

correct

> And then there is orc, which seems to count as "CPU-specific code" in this
> context.


> I guess the "force_generic_code" name was too generic for me to understand :-)
> maybe two bitflags would be even better, e g "use_cpu_specific_code" and
> "use_special_case_code", or something...

would be probably better; the force_generic thing is really just a kludge 
to case code in the test-cases (or exports)

p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)