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

[Michael Niedermayer]

On Fri, Nov 16, 2012 at 09:56:52PM +0100, Clément Bœsch wrote:
> On Thu, Nov 15, 2012 at 10:51:48PM +0100, Michael Niedermayer wrote:
> [...]
> > > > 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
> > 
> 
> The new attached patch should be less confusing.
> 
> BTW, I took the liberty to use some kind of fancy indent for the preproc,
> feel free to ask me to remove/change it; I just think it's more readable
> for such non-obvious code.
> 
> -- 
> Clément B.

>  postprocess.c                  |   98 ++++-------------
>  postprocess_altivec_template.c |    2 
>  postprocess_template.c         |  234 ++++++++++++++++++++++++-----------------
>  3 files changed, 166 insertions(+), 168 deletions(-)
> af3ee71cf339b7be826562fce3ee19ad8199120b  0001-pp-rework-the-way-templating-is-done.patch
> From 2aa01b861f7cb49f15372bd31f71a27f1e513be6 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Wed, 14 Nov 2012 20:20:35 +0100
> Subject: [PATCH] pp: rework the way templating is done.
> 
> This refactoring simplifies the usage of the template: define the
> profile and include the template is all that is required. It should now
> be easier to add more instruction sets.
> 
> The HAVE_* flags are changed with TEMPLATE_PP_* setting to avoid messing
> them up.
> 
> See the top comment in postprocess_template.c for details.

should be ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- 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/20121116/3ca7185d/attachment.asc>