Hi, On Wed, Sep 26, 2012 at 5:12 PM, Måns Rullgård <[email protected]> wrote: > "Ronald S. Bultje" <[email protected]> writes: > >> Hi, >> >> On Wed, Sep 26, 2012 at 11:00 AM, Diego Biurrun <[email protected]> wrote: >>> On Tue, Sep 11, 2012 at 10:29:08AM +0200, Diego Biurrun wrote: >>>> On Thu, Sep 06, 2012 at 11:42:45AM +0100, Måns Rullgård wrote: >>>> > "Ronald S. Bultje" <[email protected]> writes: >>>> > > On Wed, Sep 5, 2012 at 9:12 PM, Diego Biurrun <[email protected]> wrote: >>>> > >> ff_get_cpu_flags_x86() requires cpuid(), which is conditionally >>>> > >> defined >>>> > >> elsewhere in the file. Surrounding the function body with ifdefs >>>> > >> allows >>>> > >> building even when cpuid is not defined. An empty cpuflags mask is >>>> > >> returned in this case. >>>> > >> --- >>>> > >> libavutil/x86/cpu.c | 4 ++++ >>>> > >> 1 files changed, 4 insertions(+), 0 deletions(-) >>>> > >>>> > What configuration does this affect? >>>> >>>> --disable-inline-asm, see my FATE instance with that flag. >>>> >>>> > > I don't like this. I remember someone not testing his cpumask patch a >>>> > > while ago, causing all of fate to run (silently, and undetected for a >>>> > > good hour) in pure C on all x86 machines. >>>> > >>>> > That mistake had nothing to do with this patch. >>>> > >>>> > > Running in pure C should not go undetected and especially not silent. >>>> > > I prefer if this fails to compile, so we can _fix_ it instead of >>>> > > pretending to support a machine that we don't. For such machines, >>>> > > --disable-mmx or similar is good enough as a hack, right? I'm fine >>>> > > with some kind of a very loud warning also. >>>> > >>>> > There is currently only a problem if building with a compiler that >>>> > supports neither inline asm nor cpuid intrinsics. I don't know of such >>>> > a compiler. >>>> >>>> Ronald, do you still object to this patch? I believe Mans summed up >>>> that there should not be the risk you fear and it will fix a FATE >>>> instance. >>> >>> ping >> >> I asked that a warning be added to either configure output or to >> runtime (similar to the warning if you don't specifically disable asm, >> but yasm wasn't found) that you'll get useless performance. Maybe link >> it so that an explicit --disable-asm or --disable-inline-asm is >> required, and it's not possible for HAVE_YASM as well as >> HAVE_INLINE_ASM to both be disabled without specifically being told to >> do so on the configure commandline. > > That is already the case simply by virtue of HAVE_YASM having that > property. Inline asm is enabled automatically by default if the > compiler supports it.
In effect, that requires --disable-yasm to allow disabling inline asm, which sounds illogical. Maybe the warning should be modified to be more accurate? Ronald _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
