On 16 October 2015 at 03:47, James Almer <jamr...@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 <protogo...@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 <protogo...@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
