On Thu, Sep 27, 2012 at 03:07:59PM -0700, Ronald S. Bultje wrote:
> On Wed, Sep 26, 2012 at 5:12 PM, Måns Rullgård <[email protected]> wrote:
> > "Ronald S. Bultje" <[email protected]> writes:
> >> 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?
Something or somebody is confused here. First you asked for it not to
be possible for HAVE_YASM and HAVE_INLINE_ASM to both be disabled w/o
explicitly asking for such a build on the command line. Line 3109f of
configure does exactly what you require:
check_yasm "pextrd [eax], xmm0, 1" && enable yasm ||
die "yasm not found, use --disable-yasm for a crippled build"
It is possible for gcc inline asm to be disabled w/o getting a warning
or error from configure before and after this patch, so the issue is
unrelated. Thus you are asking for ... what exactly?
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
