Re: [libav-devel] [PATCH 3/9] blockdsp/x86: yasmify

2016-09-22 Thread Henrik Gramner
On Thu, Sep 22, 2016 at 9:39 AM, Anton Khirnov  wrote:
> Quoting Henrik Gramner (2016-09-21 17:13:31)
>> Why not use xorps like the original code then? INIT_XMM sse will also
>> make mova assemble to movaps instead of movdqa, so no problem there.
>
> mmx only has pxor, so I'd need yet more ifdefs then.

If you really care about optimizing for Pentium II, yes. Alternatively
just drop the MMX implementation.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/9] blockdsp/x86: yasmify

2016-09-22 Thread Anton Khirnov
Quoting Henrik Gramner (2016-09-21 17:13:31)
> On Wed, Sep 21, 2016 at 9:01 AM, Anton Khirnov  wrote:
> > Yes they are, because pxor does not exist in SSE.
> 
> Why not use xorps like the original code then? INIT_XMM sse will also
> make mova assemble to movaps instead of movdqa, so no problem there.

mmx only has pxor, so I'd need yet more ifdefs then.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/9] blockdsp/x86: yasmify

2016-09-21 Thread Henrik Gramner
On Wed, Sep 21, 2016 at 9:01 AM, Anton Khirnov  wrote:
> Yes they are, because pxor does not exist in SSE.

Why not use xorps like the original code then? INIT_XMM sse will also
make mova assemble to movaps instead of movdqa, so no problem there.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/9] blockdsp/x86: yasmify

2016-09-21 Thread Diego Biurrun
On Wed, Sep 21, 2016 at 09:01:34AM +0200, Anton Khirnov wrote:
> Quoting Diego Biurrun (2016-09-06 15:45:44)
> > On Mon, Sep 05, 2016 at 01:02:37PM +0200, Anton Khirnov wrote:
> > > --- a/libavcodec/x86/blockdsp.c
> > > +++ /dev/null
> > > @@ -1,118 +0,0 @@
> > > -av_cold void ff_blockdsp_init_x86(BlockDSPContext *c)
> > > -{
> > > -if (INLINE_MMX(cpu_flags)) {
> > > -c->clear_block  = clear_block_mmx;
> > > -c->clear_blocks = clear_blocks_mmx;
> > > -}
> > > -
> > > -#if FF_API_XVMC
> > > -FF_DISABLE_DEPRECATION_WARNINGS
> > > -/* XvMCCreateBlocks() may not allocate 16-byte aligned blocks */
> > > -if (CONFIG_MPEG_XVMC_DECODER && avctx->xvmc_acceleration > 1)
> > > -return;
> > > -FF_ENABLE_DEPRECATION_WARNINGS
> > > -#endif /* FF_API_XVMC */
> > 
> > Here the xvmc check is done after the mmx assignments ..
> > 
> > > -if (INLINE_SSE(cpu_flags)) {
> > > -c->clear_block  = clear_block_sse;
> > > -c->clear_blocks = clear_blocks_sse;
> > > -}
> > > -}
> > 
> > Here the functions are sse ..
> > 
> > > --- /dev/null
> > > +++ b/libavcodec/x86/blockdsp_init.c
> > > @@ -0,0 +1,64 @@
> > > +#include "libavutil/cpu.h"
> > > +#include "libavutil/x86/asm.h"
> > 
> > asm.h is only necessary for inline assembly code, thus not here.
> > 
> > > +av_cold void ff_blockdsp_init_x86(BlockDSPContext *c)
> > > +{
> > > +#if FF_API_XVMC
> > > +FF_DISABLE_DEPRECATION_WARNINGS
> > > +/* XvMCCreateBlocks() may not allocate 16-byte aligned blocks */
> > > +if (CONFIG_MPEG_XVMC_DECODER && avctx->xvmc_acceleration > 1)
> > > +return;
> > > +FF_ENABLE_DEPRECATION_WARNINGS
> > > +#endif /* FF_API_XVMC */
> > > +
> > > +if (EXTERNAL_MMX(cpu_flags)) {
> > > +c->clear_block  = ff_clear_block_128_mmx;
> > > +c->clear_blocks = ff_clear_block_768_mmx;
> > > +}
> > 
> > .. and here the mmx assignments come after the xvmc check.
> 
> It doesn't matter really, nobody uses xvmc anyway.

I dislike the xvmc ugliness at least as much as the next guy, but as
long as we still have it that's not really a strong argument ...

> > > +if (EXTERNAL_SSE2(cpu_flags)) {
> > > +c->clear_block  = ff_clear_block_128_sse2;
> > > +c->clear_blocks = ff_clear_block_768_sse2;
> > > +}
> > 
> > .. and here the functions are sse2.
> 
> Yes they are, because pxor does not exist in SSE.

You're changing the function's SIMD requirement w/o even mentioning that
fact in the log message.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/9] blockdsp/x86: yasmify

2016-09-21 Thread Anton Khirnov
Quoting Diego Biurrun (2016-09-06 15:45:44)
> On Mon, Sep 05, 2016 at 01:02:37PM +0200, Anton Khirnov wrote:
> > --- a/libavcodec/x86/blockdsp.c
> > +++ /dev/null
> > @@ -1,118 +0,0 @@
> > -av_cold void ff_blockdsp_init_x86(BlockDSPContext *c)
> > -{
> > -if (INLINE_MMX(cpu_flags)) {
> > -c->clear_block  = clear_block_mmx;
> > -c->clear_blocks = clear_blocks_mmx;
> > -}
> > -
> > -#if FF_API_XVMC
> > -FF_DISABLE_DEPRECATION_WARNINGS
> > -/* XvMCCreateBlocks() may not allocate 16-byte aligned blocks */
> > -if (CONFIG_MPEG_XVMC_DECODER && avctx->xvmc_acceleration > 1)
> > -return;
> > -FF_ENABLE_DEPRECATION_WARNINGS
> > -#endif /* FF_API_XVMC */
> 
> Here the xvmc check is done after the mmx assignments ..
> 
> > -if (INLINE_SSE(cpu_flags)) {
> > -c->clear_block  = clear_block_sse;
> > -c->clear_blocks = clear_blocks_sse;
> > -}
> > -}
> 
> Here the functions are sse ..
> 
> > --- /dev/null
> > +++ b/libavcodec/x86/blockdsp_init.c
> > @@ -0,0 +1,64 @@
> > +#include "libavutil/cpu.h"
> > +#include "libavutil/x86/asm.h"
> 
> asm.h is only necessary for inline assembly code, thus not here.
> 
> > +av_cold void ff_blockdsp_init_x86(BlockDSPContext *c)
> > +{
> > +#if FF_API_XVMC
> > +FF_DISABLE_DEPRECATION_WARNINGS
> > +/* XvMCCreateBlocks() may not allocate 16-byte aligned blocks */
> > +if (CONFIG_MPEG_XVMC_DECODER && avctx->xvmc_acceleration > 1)
> > +return;
> > +FF_ENABLE_DEPRECATION_WARNINGS
> > +#endif /* FF_API_XVMC */
> > +
> > +if (EXTERNAL_MMX(cpu_flags)) {
> > +c->clear_block  = ff_clear_block_128_mmx;
> > +c->clear_blocks = ff_clear_block_768_mmx;
> > +}
> 
> .. and here the mmx assignments come after the xvmc check.

It doesn't matter really, nobody uses xvmc anyway.

> 
> > +if (EXTERNAL_SSE2(cpu_flags)) {
> > +c->clear_block  = ff_clear_block_128_sse2;
> > +c->clear_blocks = ff_clear_block_768_sse2;
> > +}
> 
> .. and here the functions are sse2.

Yes they are, because pxor does not exist in SSE.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/9] blockdsp/x86: yasmify

2016-09-06 Thread Diego Biurrun
On Mon, Sep 05, 2016 at 01:02:37PM +0200, Anton Khirnov wrote:
> --- a/libavcodec/x86/blockdsp.c
> +++ /dev/null
> @@ -1,118 +0,0 @@
> -av_cold void ff_blockdsp_init_x86(BlockDSPContext *c)
> -{
> -if (INLINE_MMX(cpu_flags)) {
> -c->clear_block  = clear_block_mmx;
> -c->clear_blocks = clear_blocks_mmx;
> -}
> -
> -#if FF_API_XVMC
> -FF_DISABLE_DEPRECATION_WARNINGS
> -/* XvMCCreateBlocks() may not allocate 16-byte aligned blocks */
> -if (CONFIG_MPEG_XVMC_DECODER && avctx->xvmc_acceleration > 1)
> -return;
> -FF_ENABLE_DEPRECATION_WARNINGS
> -#endif /* FF_API_XVMC */

Here the xvmc check is done after the mmx assignments ..

> -if (INLINE_SSE(cpu_flags)) {
> -c->clear_block  = clear_block_sse;
> -c->clear_blocks = clear_blocks_sse;
> -}
> -}

Here the functions are sse ..

> --- /dev/null
> +++ b/libavcodec/x86/blockdsp_init.c
> @@ -0,0 +1,64 @@
> +#include "libavutil/cpu.h"
> +#include "libavutil/x86/asm.h"

asm.h is only necessary for inline assembly code, thus not here.

> +av_cold void ff_blockdsp_init_x86(BlockDSPContext *c)
> +{
> +#if FF_API_XVMC
> +FF_DISABLE_DEPRECATION_WARNINGS
> +/* XvMCCreateBlocks() may not allocate 16-byte aligned blocks */
> +if (CONFIG_MPEG_XVMC_DECODER && avctx->xvmc_acceleration > 1)
> +return;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif /* FF_API_XVMC */
> +
> +if (EXTERNAL_MMX(cpu_flags)) {
> +c->clear_block  = ff_clear_block_128_mmx;
> +c->clear_blocks = ff_clear_block_768_mmx;
> +}

.. and here the mmx assignments come after the xvmc check.

> +if (EXTERNAL_SSE2(cpu_flags)) {
> +c->clear_block  = ff_clear_block_128_sse2;
> +c->clear_blocks = ff_clear_block_768_sse2;
> +}

.. and here the functions are sse2.

> +#endif

Please comment the #endif.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel