On Tue, Jul 17, 2012 at 10:24:01AM +0100, Måns Rullgård wrote: > Diego Biurrun <[email protected]> writes: > > On Mon, Jul 16, 2012 at 03:28:03PM +0100, Måns Rullgård wrote: > >> Diego Biurrun <[email protected]> writes: > >> > On Mon, Jul 16, 2012 at 02:17:06PM +0100, Mans Rullgard wrote: > >> >> > >> >> --- a/libavcodec/arm/Makefile > >> >> +++ b/libavcodec/arm/Makefile > >> >> @@ -14,6 +14,7 @@ OBJS-$(CONFIG_FLAC_DECODER) += > >> >> arm/flacdsp_init_arm.o \ > >> >> OBJS-$(CONFIG_MPEGAUDIODSP) += arm/mpegaudiodsp_init_arm.o > >> >> ARMV6-OBJS-$(CONFIG_MPEGAUDIODSP) += > >> >> arm/mpegaudiodsp_fixed_armv6.o > >> >> > >> >> +OBJS-$(CONFIG_VP3DSP) += arm/vp3dsp_init_arm.o > >> >> OBJS-$(CONFIG_VP5_DECODER) += arm/vp56dsp_init_arm.o > >> > > >> > So the general strategy is to compile files containing optimizations > >> > unconditionally and then have ifdefs in the files? > >> > >> There is no "general strategy." In this specific case, the choice is > >> either adding another file just for the init function, or placing the > >> altivec code under ifdef. The ARM and x86 VP3 functions are in separate > >> .S/.asm files anyway, so there is only one option there. > >> > >> The init function has to be built unconditionally to allow calling it > >> based on ARCH_FOO only. > > > > My point is precisely to find a general strategy - there is a lot of > > inline x86 asm that will need to undergo a similar set of changes. > > The x86 asm is so messy that no single rule can ever be sensibly applied. > > > In the VP3 case we have incongruency between x86/arm and ppc in that the > > former has the code in separate files, the latter under ifdef. > > You're missing the bigger point that x86 and ARM have the code as > .asm/.S files, which cannot possibly include the init function. The PPC > code is written in C/intrinsics. Your comparison is thus meaningless. > > > What we need to decide on is how to handle such situations so that > > it's clear how to proceed in the x86 inline asm case. > > Do whatever makes more sense in each specific case. There is no need > for a single rule. > > >> > Previously we did not compile the VP3 optimizations w/o MMX > >> > available... > >> > >> MMX? This is PPC code... > > > > s/mmx/altivec/ > > > >> All the optimised functions are built under exactly the same conditions > >> as before. > > > > Yes, but we moved from conditional compilation to ifdefs. > > As I said, I did that to avoid adding a whole new file only for a 3-line > init function. If you'd rather have separate files for the init > functions, _please_ just say so instead of meandering about.
I wanted to settle this issue up front before I start making a rather large number of similar changes so as to not go in the wrong direction and be forced to redo work. No single rule is perfectly fine as an outcome, I'll apply my own best judgement in libavcodec/x86/ then. This patch is OK in principle, fix up the missing av_cold attributes you mentioned on IRC before pushing. Diego _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
