On 2015-12-31 17:00:40 +0200, Martin Storsjö wrote:
> On Thu, 31 Dec 2015, Janne Grunau wrote:
> 
> >On 2015-12-31 12:46:39 +0200, Martin Storsjö wrote:
> >>Use two separate functions, depending on whether neon is available.
> >
> >should be neon or vfp
> 
> Ah, yes, so if either NEON or VFPv3 is available (or just check for
> VFPv3?).

vfp(v2). I don't think is possible to build a CPU without VFP and with
NEON (at least using ARM's CPU design). But since it's possible to
configure with --disable-vfp --enable-neon the correct check is
'cpu_flags & (VFP | NEON)'

> >>This is set to require armv5te - it uses blx, which is only available
> >>since armv5t, but we don't have a separate configure item for that.
> >>(It also uses ldrd, which requires armv5te, but this could be avoided
> >>if necessary.)
> >
> >This breaks running checkasm on armv4 with runtime cpu detection. It's
> >not very useful since it only checks if two consecutive calls of the c
> >variant return equal/similar results.
> 
> Hmm, indeed. Shouldn't this be fixed by using HAVE_ARMV5TE_EXTERNAL
> instead?

yes, that would work

> >>diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
> >>index d6f8ffc..2c0bbbe 100644
> >>--- a/tests/checkasm/checkasm.c
> >>+++ b/tests/checkasm/checkasm.c
> >>@@ -53,6 +53,10 @@
> >> #define isatty(fd) 1
> >> #endif
> >>
> >>+#if HAVE_ARMV5TE
> >>+void (*checkasm_checked_call)(void *func, ...) = 
> >>checkasm_checked_call_noneon;
> >>+#endif
> >>+
> >> /* List of tests to invoke */
> >> static const struct {
> >>     const char *name;
> >>@@ -463,6 +467,11 @@ int main(int argc, char *argv[])
> >> {
> >>     int i, seed, ret = 0;
> >>
> >>+#if HAVE_ARMV5TE && HAVE_NEON
> >>+    if (av_get_cpu_flags() & AV_CPU_FLAG_NEON)
> >>+        checkasm_checked_call = checkasm_checked_call_neon;
> >>+#endif
> >
> >I think it would be cleaner to have this in a single ARCH_ARM block here
> 
> Hmm, can you elaborate a little? We need to have at least an ifdef
> check for HAVE_NEON (or rather VFPV3) to avoid undefined references
> - is there some existing ARCH_ARM block you suggest I should merge
> it into?

I missed the function pointer definition in the first added
'#if HAVE_ARMV5TE' so the comment is mostly void. I still would prefer
'#if ARCH_ARM && HAVE_ARMV5TE_EXTERNAL' though.

Janne
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to