[FFmpeg-devel] [PATCH] Remove useless preprocessor directives.

[Reimar Döffinger]

On Fri, Jun 18, 2010 at 08:51:29AM +0200, Benoit Fouet wrote:
> On Thu, 17 Jun 2010 21:57:09 +0200 Reimar D?ffinger wrote:
> > On Thu, Jun 17, 2010 at 03:32:02PM +0200, Benoit Fouet wrote:
> > > yuv420_bgr32 and yuva420_bgr32 are only used if HAVE_7REGS is set.
> > > The other solution would be to add an #else case.
> > > 
> > > This fixes the following warnings:
> > > In file included from libswscale/x86/yuv2rgb_mmx.c:55:
> > > libswscale/x86/yuv2rgb_template.c: In function ?yuva420_rgb32_MMX?:
> > > libswscale/x86/yuv2rgb_template.c:410: warning: no return statement in function returning non-void
> > > libswscale/x86/yuv2rgb_template.c: In function ?yuva420_bgr32_MMX?:
> > > libswscale/x86/yuv2rgb_template.c:453: warning: no return statement in function returning non-void
> > > In file included from libswscale/x86/yuv2rgb_mmx.c:62:
> > > libswscale/x86/yuv2rgb_template.c: In function ?yuva420_rgb32_MMX2?:
> > > libswscale/x86/yuv2rgb_template.c:410: warning: no return statement in function returning non-void
> > > libswscale/x86/yuv2rgb_template.c: In function ?yuva420_bgr32_MMX2?:
> > > libswscale/x86/yuv2rgb_template.c:453: warning: no return statement in function returning non-void
> > > ---
> > >  x86/yuv2rgb_template.c |    4 ----
> > >  1 files changed, 0 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/x86/yuv2rgb_template.c b/x86/yuv2rgb_template.c
> > > index 5c062c1..d91c941 100644
> > > --- a/x86/yuv2rgb_template.c
> > > +++ b/x86/yuv2rgb_template.c
> > > @@ -391,7 +391,6 @@ static inline int RENAME(yuva420_rgb32)(SwsContext *c, const uint8_t *src[],
> > >                                          int srcSliceY, int srcSliceH,
> > >                                          uint8_t *dst[], int dstStride[])
> > >  {
> > > -#if HAVE_7REGS
> > >      int y, h_size;
> > >  
> > >      YUV2RGB_LOOP(4)
> > > @@ -406,7 +405,6 @@ static inline int RENAME(yuva420_rgb32)(SwsContext *c, const uint8_t *src[],
> > >      YUV2RGB_ENDLOOP(4)
> > >      YUV2RGB_OPERANDS_ALPHA
> > >      YUV2RGB_ENDFUNC
> > > -#endif
> > >  }
> > >  
> > >  static inline int RENAME(yuv420_bgr32)(SwsContext *c, const uint8_t *src[],
> > > @@ -434,7 +432,6 @@ static inline int RENAME(yuva420_bgr32)(SwsContext *c, const uint8_t *src[],
> > >                                          int srcSliceY, int srcSliceH,
> > >                                          uint8_t *dst[], int dstStride[])
> > >  {
> > > -#if HAVE_7REGS
> > >      int y, h_size;
> > >  
> > >      YUV2RGB_LOOP(4)
> > > @@ -449,5 +446,4 @@ static inline int RENAME(yuva420_bgr32)(SwsContext *c, const uint8_t *src[],
> > >      YUV2RGB_ENDLOOP(4)
> > >      YUV2RGB_OPERANDS_ALPHA
> > >      YUV2RGB_ENDFUNC
> > > -#endif
> > 
> > This is complete nonsense, HAVE_7REGS etc. are used because the corresponding
> > code can't _compile_, if it did compile it would work fine and we could use it.
> 
> Let's make things clear about nonsense: I had the warning without the
> patch (meaning that 7REGS is not set on my machine); I don't have it
> with the patch. So yes, of course, I did compile!

The reason why I said nonsense is because it is either way:
If it actually compiles, then you should also remove the if (),
because there is no reason not to use that function if we managed
to compile it.
Unless you meant to say your gcc does dead-code-elimination and
removal of unused functions before register allocation for asm?

> diff --git a/x86/yuv2rgb_template.c b/x86/yuv2rgb_template.c
> index 5c062c1..73e475c 100644
> --- a/x86/yuv2rgb_template.c
> +++ b/x86/yuv2rgb_template.c
> @@ -406,6 +406,8 @@ static inline int RENAME(yuva420_rgb32)(SwsContext *c, const uint8_t *src[],
>      YUV2RGB_ENDLOOP(4)
>      YUV2RGB_OPERANDS_ALPHA
>      YUV2RGB_ENDFUNC
> +#else
> +    return 0; /* Cannot happen */
>  #endif

You could e.g. just put the YUV2RGB_ENDFUNC below the endif...