[Ffmpeg-devel] [PATCH] Getting rid of inlining failure warnings
Michael Niedermayer
michaelni
Thu Nov 9 16:32:56 CET 2006
Hi
On Thu, Nov 09, 2006 at 03:47:08PM +0100, Panagiotis Issaris wrote:
> Hi,
>
> On Thu, 2006-11-09 at 14:59 +0100, Panagiotis Issaris wrote:
> > [...]
> > I'll send a new patch later in which I will send smaller patches,
> > removing the inline specifier only if no new warnings are introduced by
> > doing so.
>
> The attached patch gets rid of inlining failure warnings.
>
> Total number of warnings before the patch: 1141
> Number of inlining warning before the patch: 353
>
> Total number of warnings after the patch: 579
> Number of inlining warning after the patch: 72
>
>
> asv1.c | 6 +++---
> cavs.c | 2 +-
> ffv1.c | 4 ++--
> golomb.h | 2 +-
> h263.c | 8 ++++----
> h264.c | 6 +++---
> jpeg_ls.c | 4 ++--
> motion_est.c | 6 +++---
> motion_est_template.c | 4 ++--
> mpeg12.c | 6 +++---
> mpegvideo.c | 14 +++++++-------
> mpegvideo.h | 4 ++--
> msmpeg4.c | 8 ++++----
> snow.c | 6 +++---
> svq3.c | 2 +-
> vc1.c | 2 +-
> 16 files changed, 42 insertions(+), 42 deletions(-)
>
> Regression tests succeed. As inlining already failed, removing the
> specifier should have no performance impact (at least on all compilers
> where the same inlining warnings would have appeared...)
the warnings often are specific to function X in Y and dont mean X is never
inlined
[...]
> Index: libavcodec/ffv1.c
> ===================================================================
> --- libavcodec/ffv1.c (revision 6954)
> +++ libavcodec/ffv1.c (working copy)
> @@ -221,7 +221,7 @@
> return f->quant_table[0][(L-LT) & 0xFF] + f->quant_table[1][(LT-T) & 0xFF] + f->quant_table[2][(T-RT) & 0xFF];
> }
>
> -static inline void put_symbol(RangeCoder *c, uint8_t *state, int v, int is_signed){
> +static void put_symbol(RangeCoder *c, uint8_t *state, int v, int is_signed){
the one call to put_symbol in encode_line should always be inlined all others
should never be inlined (a noinline warper around a always_inline
put_symbol() should do the trick)
> int i;
>
> if(v){
> @@ -357,7 +357,7 @@
> }
>
> #ifdef CONFIG_ENCODERS
> -static inline int encode_line(FFV1Context *s, int w, int_fast16_t *sample[2], int plane_index, int bits){
> +static int encode_line(FFV1Context *s, int w, int_fast16_t *sample[2], int plane_index, int bits){
used only once per fileformat case -> always_inline
and try attriute(flatten) or change gccs inlining limits if needed, but the
bits parameter is a constant and used in the inner loop, the same is true
for jpeg_ls, so these may be faster if inlined (and nothing else is outlined
by gcc)
[...]
> Index: libavcodec/jpeg_ls.c
> ===================================================================
> --- libavcodec/jpeg_ls.c (revision 6954)
> +++ libavcodec/jpeg_ls.c (working copy)
> @@ -288,7 +288,7 @@
> /**
> * Decode one line of image
> */
> -static inline void ls_decode_line(JLSState *state, MJpegDecodeContext *s, void *last, void *dst, int last2, int w, int stride, int comp, int bits){
> +static void ls_decode_line(JLSState *state, MJpegDecodeContext *s, void *last, void *dst, int last2, int w, int stride, int comp, int bits){
used just once per fileformat -> always_inline
[...]
> @@ -577,7 +577,7 @@
> /**
> * Encode one line of image
> */
> -static inline void ls_encode_line(JLSState *state, PutBitContext *pb, void *last, void *cur, int last2, int w, int stride, int comp, int bits){
> +static void ls_encode_line(JLSState *state, PutBitContext *pb, void *last, void *cur, int last2, int w, int stride, int comp, int bits){
used just once per fileformat -> always_inline
[...]
> Index: libavcodec/mpegvideo.c
> ===================================================================
> --- libavcodec/mpegvideo.c (revision 6954)
> +++ libavcodec/mpegvideo.c (working copy)
> @@ -3438,7 +3438,7 @@
> * @param pic_op qpel motion compensation function (average or put normally)
> * the motion vectors are taken from s->mv and the MV type from s->mv_type
> */
> -static inline void MPV_motion(MpegEncContext *s,
> +static void MPV_motion(MpegEncContext *s,
this and the lowres case ok
btw you could try to factor the s->mv[dir] in these functions out, like:
mvdir= s->mv[dir] at the top of the functions, this might make them faster or
even pass mvdir as a parameter to them, as dir is a constant in all calls
[...]
> @@ -4121,7 +4121,7 @@
>
> #ifdef CONFIG_ENCODERS
>
> -static inline void dct_single_coeff_elimination(MpegEncContext *s, int n, int threshold)
> +static void dct_single_coeff_elimination(MpegEncContext *s, int n, int threshold)
noone uses single_coeff_elimination so ok
> {
> static const char tab[64]=
> {3,2,2,1,1,1,1,1,
> @@ -4170,7 +4170,7 @@
> else s->block_last_index[n]= -1;
> }
>
> -static inline void clip_coeffs(MpegEncContext *s, DCTELEM *block, int last_index)
> +static void clip_coeffs(MpegEncContext *s, DCTELEM *block, int last_index)
called rarely so ok
[...]
> @@ -4636,7 +4636,7 @@
> put_bits(pb, bits, be2me_16(srcw[words])>>(16-bits));
> }
>
> -static inline void copy_context_before_encode(MpegEncContext *d, MpegEncContext *s, int type){
> +static void copy_context_before_encode(MpegEncContext *d, MpegEncContext *s, int type){
hmm, iam unsure, if this should be inlined or not, id say leave it
[...]
> @@ -4662,7 +4662,7 @@
> d->dquant= s->dquant;
> }
>
> -static inline void copy_context_after_encode(MpegEncContext *d, MpegEncContext *s, int type){
> +static void copy_context_after_encode(MpegEncContext *d, MpegEncContext *s, int type){
hmm, iam unsure, if this should be inlined or not, id say leave it
[...]
> @@ -4699,7 +4699,7 @@
> d->qscale= s->qscale;
> }
>
> -static inline void encode_mb_hq(MpegEncContext *s, MpegEncContext *backup, MpegEncContext *best, int type,
> +static void encode_mb_hq(MpegEncContext *s, MpegEncContext *backup, MpegEncContext *best, int type,
> PutBitContext pb[2], PutBitContext pb2[2], PutBitContext tex_pb[2],
> int *dmin, int *next_block, int motion_x, int motion_y)
ok, this is called from too many spots
ill look at the stuff after mpegvideo* later, the above should be
dealt with and benchmarked first IMHO
to benchmark START/STOP_TIMER at appropriate places should be used
appropriate is not to close and not to far away from the code
one thing you could try is around avcodec_en/decode_video() ...
but that might be too far away to detect small differences reliably
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is
More information about the ffmpeg-devel
mailing list