On 2016-06-13 21:04:15 +0300, Martin Storsjö wrote:
> On Mon, 13 Jun 2016, Diego Biurrun wrote:
> 
> >This avoids SIMD-optimized functions having to sign-extend their
> >line size argument manually to be able to do pointer arithmetic.
> >---
> >
> >Added missing changes for h264idct.
> >
> >libavcodec/aarch64/h264dsp_init_aarch64.c | 37 +++++++++++------------
> >libavcodec/arm/h264dsp_init_arm.c         | 37 +++++++++++------------
> >libavcodec/h264_mb.c                      |  6 ++--
> >libavcodec/h264_mb_template.c             |  2 +-
> >libavcodec/h264addpx_template.c           |  4 +--
> >libavcodec/h264dsp.h                      | 49 
> >++++++++++++++++---------------
> >libavcodec/h264dsp_template.c             | 36 +++++++++++------------
> >libavcodec/h264idct.h                     | 19 ++++++------
> >libavcodec/h264idct_template.c            | 30 ++++++++++++-------
> >libavcodec/ppc/h264dsp.c                  | 31 +++++++++----------
> >libavcodec/x86/h264_deblock.asm           | 20 ++++++-------
> >libavcodec/x86/h264_deblock_10bit.asm     | 14 ++++-----
> >libavcodec/x86/h264_idct.asm              | 48 +++++++++++++++---------------
> >libavcodec/x86/h264_idct_10bit.asm        | 18 ++++++------
> >libavcodec/x86/h264_weight.asm            |  4 +--
> >libavcodec/x86/h264_weight_10bit.asm      |  4 +--
> >libavcodec/x86/h264dsp_init.c             | 22 +++++++-------
> >tests/checkasm/h264dsp.c                  |  4 +--
> >18 files changed, 199 insertions(+), 186 deletions(-)
> 
> This doesn't do the actual cleanup of the asm that I had expected.
> 
> For x86_64, you should remove the corresponding movsxd/movsxdifnidn, for
> aarch64, you need to change use of wN (e.g. w3) into xN (e.g. x4) for those
> parameters. If not, we're basically clipping the variable and just reading
> the lower 32 bit, which is just about as wrong as what we did before
> (although in practice it will probably be fine).
> 
> Do we have a policy about how to do this, is asm cleanup later ok? What does
> e.g. James and Janne think about that for x86 and aarch64 respectively?

I'd prefer corrosponding chnages to C and asm code in the same commit

> Also, this lacks the commit message that we discussed, pointing out 
> that
> h264idct was lacking the necessary sign extension before.

I think we should fix the x86 asm first and then change the stride type

Janne
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to