On Thu, 10 Dec 2015, Janne Grunau wrote:

On 2015-12-10 15:39:05 +0200, Martin Storsjö wrote:
On Wed, 9 Dec 2015, Janne Grunau wrote:

The vector mode was deprecated in ARMv7-A/VFPv3 and various cpu
implementations do not support it in hardware. Vector mode code will
depending the OS either be emulated in software or result in an illegal
instruction on cpus which does not support it. This was not really
problem in practice since NEON implementations of the same functions are
preferred. It will however become a problem for checkasm which tests
every cpu flag separately.

Since this is a cpu feature newer cpu do not support anymore the
behaviour of this flag differs from the other flags. It can be only
activated by runtime cpu feature selection.
---
libavutil/arm/cpu.h                  | 4 ++++
libavutil/cpu.h                      | 1 +
libavcodec/arm/dcadsp_init_arm.c     | 4 ++--
libavcodec/arm/fft_init_arm.c        | 2 +-
libavcodec/arm/fmtconvert_init_arm.c | 2 +-
libavutil/arm/cpu.c                  | 4 ++++
libavutil/cpu.c                      | 2 ++
tests/checkasm/checkasm.c            | 1 +
8 files changed, 16 insertions(+), 4 deletions(-)

This seems ok. Should something else be done for the cases where we
lack runtime detection (like old ARMv6 iOS)? I.e. add something
similar within the non-runtime version of ff_get_cpu_flags_arm?

I'm not sure if that makes much sense. We could of course add it to
configure with vfp_vm_deps="vfp !vfp3 !neon". The problem with that
approach is that it will always use the vfp vector mode asm functions
even when it runs on armv7-a cpu which doesn't support it.

This doesn't seem to be a problem in practice as it is the current
behaviour but feels wrong. One option would be not to enable it by
default but I guess that means nobody will discover and use the option.

What about HW detection for ios? I see that
sysctl({CTL_HW,HW_MACHINE},...) returns the "iPhone2,1" type model
names. Parsing them is annoying so it's worth checking if HW_MODEL or
HW_MACHINE_ARCH returns easier to parse information. sysctl() should be
allowed, we use it already to detect the number of CPUs and I found
articles that getting the process list via sysctl was specifically
disabled in ios 9. Parsing the model strings is not that much uglier
than parsing /proc/cpuinfo and the cpu flags are cached anyway.

I'd rather skip runtime detection for iOS, since it has different binary slices, which is quite enough in almost all cases. (I didn't even think of the chance of running the armv6 version of such a binary on armv7 devices in my previous mail.)

Given this, I think your current patch is fine. It's all academic anyway, since one can't publish ARMv6 binaries for iOS any longer, and other platforms lacking runtime detection currently (like windows phone) don't run on armv6 anyway (or like the older windows mobile, probably mostly run on even older processors).

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

Reply via email to