[FFmpeg-devel] [PATCH 4/6] pp: move optim templating config to template itself.

[Clément Bœsch]

On Thu, Nov 15, 2012 at 01:10:51AM +0100, Michael Niedermayer wrote:
> On Wed, Nov 14, 2012 at 11:29:58PM +0100, Clément Bœsch wrote:
[...]
> > +#ifdef COMPILE_ALTIVEC
> > +# define COMPILE_ALTIVEC 1
> 
> this looks wrong
> 

Oups, indeed, will fix.

> 
> > +# define RENAME(a) a ## _altivec
> > +#else
> > +# define COMPILE_ALTIVEC 0
> > +#endif
> > +
> > +#ifdef COMPILE_MMX
> > +# define RENAME(a) a ## _MMX
> > +#else
> > +# define COMPILE_MMX 0
> > +#endif
> > +
> > +#ifdef COMPILE_MMXEXT
> > +# undef  COMPILE_MMX
> > +# define COMPILE_MMX 1
> > +# define RENAME(a) a ## _MMX2
> > +#else
> > +# define COMPILE_MMXEXT 0
> > +#endif
> > +
> > +#ifdef COMPILE_3DNOW
> > +# undef  COMPILE_MMX
> > +# define COMPILE_MMX 1
> > +# define RENAME(a) a ## _3DNow
> > +#else
> > +# define COMPILE_3DNOW 0
> > +#endif
> > +
> >  #undef REAL_PAVGB
> >  #undef PAVGB
> >  #undef PMINUB
> >  #undef PMAXUB
> >  
> > -#if   HAVE_MMXEXT_INLINE
> > +#if   COMPILE_MMXEXT
> >  #define REAL_PAVGB(a,b) "pavgb " #a ", " #b " \n\t"
> > -#elif HAVE_AMD3DNOW_INLINE
> > +#elif COMPILE_3DNOW
> >  #define REAL_PAVGB(a,b) "pavgusb " #a ", " #b " \n\t"
> >  #endif
> >  #define PAVGB(a,b)  REAL_PAVGB(a,b)
> >  
> > -#if   HAVE_MMXEXT_INLINE
> > +#if   COMPILE_MMXEXT
> >  #define PMINUB(a,b,t) "pminub " #a ", " #b " \n\t"
> > -#elif HAVE_MMX_INLINE
> > +#elif COMPILE_MMX
> >  #define PMINUB(b,a,t) \
> >      "movq " #a ", " #t " \n\t"\
> >      "psubusb " #b ", " #t " \n\t"\
> >      "psubb " #t ", " #a " \n\t"
> >  #endif
> >  
> > -#if   HAVE_MMXEXT_INLINE
> > +#if   COMPILE_MMXEXT
> >  #define PMAXUB(a,b) "pmaxub " #a ", " #b " \n\t"
> > -#elif HAVE_MMX_INLINE
> > +#elif COMPILE_MMX
> >  #define PMAXUB(a,b) \
> >      "psubusb " #a ", " #b " \n\t"\
> >      "paddb " #a ", " #b " \n\t"
> >  #endif
> >  
> >  //FIXME? |255-0| = 1 (should not be a problem ...)
> > -#if HAVE_MMX_INLINE
> > +#if COMPILE_MMX
> 
> These changes dont feel correct
> For the template in its instance of compilation the question is
> do i HAVE MMX/MMX2/3dnow/SSE on the target i get compiled for
> 
> or to say it differently
> one could compile for MMX2 but not MMX in this case COMPILE_MMX2
> would be set but not MMX. Yet the template should have its define
> for MMX+MMX2 set. Aka i get COMPILED for MMX2 and HAVE MMX+MMX2
> 

I don't understand: isn't the original code adding a MMX "dependency" in
the MMX2 templating mode? Reading this:

    //MMX versions
    #ifdef COMPILE_MMX
    #undef RENAME
    #undef HAVE_MMX_INLINE
    #define HAVE_MMX_INLINE 1
    #define RENAME(a) a ## _MMX
    #include "postprocess_template.c"
    #endif

    //MMX2 versions
    #ifdef COMPILE_MMX2
    #undef RENAME
    #undef HAVE_MMX_INLINE
    #undef HAVE_MMXEXT_INLINE
    #define HAVE_MMX_INLINE 1
    #define HAVE_MMXEXT_INLINE 1
    #define RENAME(a) a ## _MMX2
    #include "postprocess_template.c"
    #endif

...I assumed MMX2 template couldn't work with MMX disabled, so I kept that
logic.

> If you use COMPILE_* for this then COMPILE_* has a different semantic
> meaning inside and outside of teh template, this appears bad to me.
> 

Indeed, the dependency logic is moved to the template itself. So if you
request a "MMX2 compile" it will indeed compile MMX as well. If it's not
a technical issue but only a semantic one, I don't mind renaming
"COMPILE_" to "TEMPLATE_" (or any other suggestion). I just wanted to
avoid overriding the HAVE_* flags.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121115/6b3ed4a9/attachment.asc>