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

[Michael Niedermayer]

On Thu, Nov 15, 2012 at 10:24:54PM +0100, Clément Bœsch wrote:
> 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.

its probably just a semantic issue, cant say for sure,the reuse of
the COMPILE_* could have hidden some other issue

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121115/fbf3d20e/attachment.asc>