Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Rémi Denis-Courmont" <remi@remlab.net>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCHv4 2/4] lavc/mpegvideo: use H263DSP dequant function
Date: Tue, 11 Jun 2024 18:10:22 +0300
Message-ID: <1759316.BgMm0vUi2E@basile.remlab.net> (raw)
In-Reply-To: <GV1P250MB073738769A0BC5C331F15AE68FC72@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM>

Le tiistaina 11. kesäkuuta 2024, 16.00.28 EEST Andreas Rheinhardt a écrit :
> Rémi Denis-Courmont:
> > The same as any other indirect function tail call. What do you want me to
> > say?! Changing how FFmpeg handles assembler optimisations, or removing
> > the upper level of indirection is outside the scope of this MR (and not
> > very realistic).
> > 
> > For that matter, there are hundreds of indirect calls that could be
> > optimised out on some platforms, notably almost all Armv8 NEON functions
> > and, on x86-64, all MMX or SSE2 exclusives. I don't get nitpicking this
> > one.

> 1. Given that we always build the C versions of dsp functions, the
> latter claim is wrong.

Of course not. The C function are being compiled in pointlessly (leaving aside 
checkasm), taking up code size and adding unnecessary indirection. Of course 
assembler functions cannot be inlined, but they could very well use *direct* 
and trivially branch-predicted calls.

> 2. I am not specifically asking about the cost of the indirect call, but
> of the call. The callee will only be called from one place in this
> scenario (presuming (for the C versions) that the compiler inlines
> h263_dct_unquantize_inter_c into h263_dct_unquantize_intra_c), so if it
> were not for arch-specific optimized versions we would inline it in its
> caller (just as it is now).

The outer function itself is address-taken and therefore cannot be inlined. A 
a consequence, the block argument is to be in a register on entry anyway. The 
qmul and qadd are very obviously going to go into registers too either way 
since the compiler cannot assume what their value is (at best, it could make a 
special case for qadd=0). And obviously the counter is going to be in a 
register too. So the overhead is really the function call per se.

And so I don't get what you are asking for.

> The loop does not look like it benefits a
> lot from vectorizing, so I am really wondering whether this optimization
> is even a win when benchmarking the real thing (not only the loop).

There are existing optimisations for x86, AArch64, Arm (2x), MIPS (2x), Alpha 
and PowerPC. They all use inline assembler or compiler intrinsics to be able 
to replicate the non-trivial scalar prologue in C. I can replicate those anti-
patterns for RISC-V, and give up on the checkasm tests.

Needless to say that that seems way worse as far as code quality is concerned.

-- 
Rémi Denis-Courmont
http://www.remlab.net/



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2024-06-11 15:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-10 20:23 [FFmpeg-devel] [PATCHv4 1/4] lavc/h263dsp: add DCT dequantisation functions Rémi Denis-Courmont
2024-06-10 20:23 ` [FFmpeg-devel] [PATCHv4 2/4] lavc/mpegvideo: use H263DSP dequant function Rémi Denis-Courmont
2024-06-11 10:37   ` Andreas Rheinhardt
2024-06-11 12:02     ` Rémi Denis-Courmont
2024-06-11 13:00       ` Andreas Rheinhardt
2024-06-11 15:10         ` Rémi Denis-Courmont [this message]
2024-06-10 20:23 ` [FFmpeg-devel] [PATCHv4 3/4] checkasm/h263dsp: test dct_unquantize_{intra, inter} Rémi Denis-Courmont
2024-06-12 17:34   ` James Almer
2024-06-10 20:23 ` [FFmpeg-devel] [PATCHv4 4/4] lavc/h263dsp: R-V V " Rémi Denis-Courmont
2024-06-11 10:30 ` [FFmpeg-devel] [PATCHv4 1/4] lavc/h263dsp: add DCT dequantisation functions Andreas Rheinhardt
2024-06-12  3:41   ` Rémi Denis-Courmont
2024-06-12  3:58     ` Andreas Rheinhardt
2024-06-12 17:28 ` James Almer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1759316.BgMm0vUi2E@basile.remlab.net \
    --to=remi@remlab.net \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git