On Wed, 15 Jun 2016, Janne Grunau wrote:

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

That's my opinion as well, first fixing the few omissions in the currently almost-consistent code, then later changing it to an (hopefully) easier version.

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

Reply via email to