[FFmpeg-devel] [PATCH 1/2] lavu/intmath.h: Add msvc/icl ctzll optimisations.

Matt Oliver protogonoi at gmail.com
Fri Oct 16 00:25:57 CEST 2015


On 16 October 2015 at 03:47, James Almer <jamrial at gmail.com> wrote:

> On 10/15/2015 11:07 AM, Matt Oliver wrote:
> > This patch just adds the msvc and icl equivalent ctzll optimisations to
> > correspond with the recently added gcc variant
> >
> >
> > 0001-lavu-intmath.h-Add-msvc-icl-ctzll-optimisations.patch
> >
> >
> > From 2b8e7757cdb8595181a01bb18756d60662c41fde Mon Sep 17 00:00:00 2001
> > From: Matt Oliver <protogonoi at gmail.com>
> > Date: Thu, 15 Oct 2015 19:42:31 +1100
> > Subject: [PATCH 1/2] lavu/intmath.h: Add msvc/icl ctzll optimisations.
> >
> > Signed-off-by: Matt Oliver <protogonoi at gmail.com>
> > ---
> >  libavutil/x86/intmath.h | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/libavutil/x86/intmath.h b/libavutil/x86/intmath.h
> > index fefad20..613903b 100644
> > --- a/libavutil/x86/intmath.h
> > +++ b/libavutil/x86/intmath.h
> > @@ -60,6 +60,34 @@ static av_always_inline av_const unsigned
> av_mod_uintp2_bmi2(unsigned a, unsigne
> >
> >  #endif /* __BMI2__ */
> >
> > +#elif defined(_MSC_VER)
>
> Don't use elif if this code is also supposed to work on icl. That compiler
> defines __GNUC__.
>

ICC defines __GNUC__ but ICL does not! ICC (being the linux variant)
supports all the same functions as gcc so theres no need in this case for
special handling. However ICL defines _MSC_VER so this is correct.


>
> > +#define ff_ctzll ff_ctzll_c
> > +static av_always_inline av_const int ff_ctzll_c(long long v)
>
> nit: ff_ctzll_x86 for the inline version. And you need to check
> HAVE_INLINE_ASM for it as
> well.
>

I didnt put the HAVE_INLINE_ASM check as at this point we know its ICL and
as ICL always has inline support so I assumed it was redundant. Given that
is there actually a need to check it?


>
> > +{
> > +#if defined(__INTEL_COMPILER)
> > +#if ARCH_X86_64
> > +    uint64_t c;
> > +    __asm__("bsfq %1,%0" : "=r" (c) : "r" (v));
> > +    return c;
> > +#else
> > +    return ((uint32_t)v == 0) ? _bit_scan_forward(((uint32_t *)&v)[1])
> + 32 : _bit_scan_forward((uint32_t)v);
>
> (uint32_t)(v >> 32)
>
> > +#endif
> > +#else
> > +    unsigned long c;
> > +#if ARCH_X86_64
> > +    _BitScanForward64(&c, v);
> > +#else
> > +    if ((uint32_t)v == 0) {
> > +        _BitScanForward(&c, ((uint32_t *)&v)[1]);
>
> Same
>
> > +        c += 32;
> > +    } else {
> > +        _BitScanForward(&c, (uint32_t)v);
> > +    }
> > +#endif
> > +    return c;
> > +#endif
> > +}
> > +
> >  #endif /* __GNUC__ */
> >
> >  #endif /* AVUTIL_X86_INTMATH_H */
> > -- 1.9.5.github.0
> >
>

The other suggestions look ok so ill rewrite a new patch later today.


More information about the ffmpeg-devel mailing list