Re: [PATCH SBC 1/3] sbc: Add initial code for SSE primitives

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Marcel,

On Wed, Aug 12, 2020 at 5:48 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> >
> > ---
> > Makefile.am              |   1 +
> > sbc/sbc_primitives.c     |  20 ++-
> > sbc/sbc_primitives_sse.c | 361 +++++++++++++++++++++++++++++++++++++++
> > sbc/sbc_primitives_sse.h |  38 +++++
> > 4 files changed, 417 insertions(+), 3 deletions(-)
> > create mode 100644 sbc/sbc_primitives_sse.c
> > create mode 100644 sbc/sbc_primitives_sse.h
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 342043d..7ff0c7d 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -14,6 +14,7 @@ sbc_headers = sbc/sbc.h
> >
> > sbc_sources = sbc/sbc.c sbc/sbc_private.h sbc/sbc_math.h sbc/sbc_tables.h \
> >               sbc/sbc_primitives.h sbc/sbc_primitives.c \
> > +             sbc/sbc_primitives_sse.h sbc/sbc_primitives_sse.c \
> >               sbc/sbc_primitives_mmx.h sbc/sbc_primitives_mmx.c \
> >               sbc/sbc_primitives_iwmmxt.h sbc/sbc_primitives_iwmmxt.c \
> >               sbc/sbc_primitives_neon.h sbc/sbc_primitives_neon.c \
> > diff --git a/sbc/sbc_primitives.c b/sbc/sbc_primitives.c
> > index ff343cf..97a75be 100644
> > --- a/sbc/sbc_primitives.c
> > +++ b/sbc/sbc_primitives.c
> > @@ -33,6 +33,7 @@
> > #include "sbc_tables.h"
> >
> > #include "sbc_primitives.h"
> > +#include "sbc_primitives_sse.h"
> > #include "sbc_primitives_mmx.h"
> > #include "sbc_primitives_iwmmxt.h"
> > #include "sbc_primitives_neon.h"
> > @@ -590,6 +591,21 @@ static int sbc_calc_scalefactors_j(
> >       return joint;
> > }
> >
> > +static void sbc_init_primitives_x86(struct sbc_encoder_state *state)
> > +{
> > +     __builtin_cpu_init();
> > +
> > +#ifdef SBC_BUILD_WITH_MMX_SUPPORT
> > +     if (__builtin_cpu_supports("mmx"))
> > +             sbc_init_primitives_mmx(state);
> > +#endif
> > +
> > +#ifdef SBC_BUILD_WITH_SSE_SUPPORT
> > +     if (__builtin_cpu_supports("sse4.2"))
> > +             sbc_init_primitives_sse(state);
> > +#endif
>
> lets keep the ifdef in the primitive functions they belong. This should be consistent across all primitives and not spread in two places.

I guess you mean moving #ifdef SBC_BUILD_WITH_SSE_SUPPORT into
sbc_primitives_sse.c, same for mmx, right? I will fix that, although I
was thinking on not even compiling those files when the config option
is not enabled which would result in undefined symbols, I can however
have alternative versions when its options are not enabled which
basically does nothing.

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz