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

[Benoit Fouet]

On Fri, 18 Jun 2010 17:36:14 +0200 Reimar D?ffinger wrote:
> 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.

OK, sorry I misunderstood your tone then.

> Unless you meant to say your gcc does dead-code-elimination and
> removal of unused functions before register allocation for asm?
> 

I've just tried to remove the if () and everything compiles and link fine.

======= ><8 =======
diff --git a/x86/yuv2rgb_mmx.c b/x86/yuv2rgb_mmx.c
index 6478311..1e13733 100644
--- a/x86/yuv2rgb_mmx.c
+++ b/x86/yuv2rgb_mmx.c
@@ -35,6 +35,9 @@
 #include "libswscale/swscale_internal.h"
 #include "libavutil/x86_cpu.h"
 
+#undef HAVE_7REGS
+#define HAVE_7REGS 1
+
 #define DITHER1XBPP // only for MMX
 
 /* hope these constant values are cache line aligned */
======= ><8 =======

gcc version 4.4.3 (Ubuntu 4.4.3-4ubuntu5)

> > 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...

When considering the options I had, I eliminated this one, because I
found it was too much a hack :)

Ben