Re: [libav-devel] [PATCH] arm: Avoid functions using the deprecated 'setend' instruction on modern arches

2014-07-07 Thread Janne Grunau
On 2014-07-06 17:48:14 +0300, Martin Storsjö wrote:
 On Sun, 6 Jul 2014, Janne Grunau wrote:
 
 On 2014-07-04 18:27:54 +0300, Martin Storsjö wrote:
 
 [1] 
 http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268504.html
 ---
 
 I don't think this is a good solution for the almost non-problem.
 'setend' is deprecated but still available. It can be disabled through a
 system control register but I can't see a reason why anyone should
 disable it especially if 32-bit compatibility is important.
 
 Hmm, right. I tried to read through the whole thread I linked to but
 most of it focused around the swp instruction, so I didn't get
 complete clarity on how much of an issue this really was wrt setend
 - clearly they had picked up on its use and found it worthy of
 pointing out, and also more or less suggesting that we should stop
 using it - although it hasn't been requested directly so far.
 
 Was the point that the instruction is supposed to be disabled on
 vanilla ARMv8 kernels and the android kernels would want to behave
 as closely to the vanilla kernels as possible, thus wanting to
 change this part in the ARMv8 kernel abi?

The kernel currently does nothing with those system control register 
bits (ITD, SED and CP15BEN) but their state after reset is 
architecturally unknown (implementation defined). To rely on it either 
the kernel or bootloader has to set them.

 Yeah... Disabling if running on = ARMv8 would perhaps make more
 sense though, as it is deprecated only there.

The only problem is to determine the architecture version. I fear there 
is no cleaner way than parsing AT_PLATFORM from /proc/self/auxv or CPU 
architecture from /proc/cpuinfo.

 If we care about armv7 instructions which are deprecated on armv8 the
 partial deprecation of 'it' is even more annoying. The non-deprecated
 'it' on armv8 allows just a single conditional instruction. We use the
 deprecated form in 15 files (grep for ' it[te]+ '). Thumb2 is afaik
 default on android and ios.
 
 Indeed. This was only touched upon briefly in the discussion, and
 IIRC it was concluded that it will have to be supported more
 significantly since it's the default generated by compilers.
 
 At least on ios both deprecated 32-bit instructions are not a problem
 and I wouldn't expect it to be a problem on android either for the
 foreseeable future (as long as both allow installation of 32-bit apps on
 armv8 devices).
 
 At least in practice I guess some support for it will be kept in the
 android kernels, regardless of what the vanilla kernels do (and I
 guess that was the main point of the linked discussion, on
 convincing the vanilla kernel to commit to keeping the same
 compatibility). Hard to say for sure though so far, given the lack
 of such devices.

I think even the vanilla kernel will keep supporting those as long as 
the CPU implementation supports it. They won't disable the deprecated 
instructions. The discussion revolves around emulating deprecated 
instructions in the kernel when they are not supported by the hardware.  
I asked for a better way to detect whether deprecatd instructions can be 
used.

The solution for the deprecated IT might be forcing arm execution state 
for the functions/files using it.

Janne

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] arm: Avoid functions using the deprecated 'setend' instruction on modern arches

2014-07-07 Thread Martin Storsjö

On Mon, 7 Jul 2014, Janne Grunau wrote:


On 2014-07-06 17:48:14 +0300, Martin Storsjö wrote:

On Sun, 6 Jul 2014, Janne Grunau wrote:


On 2014-07-04 18:27:54 +0300, Martin Storsjö wrote:


[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268504.html
---


I don't think this is a good solution for the almost non-problem.
'setend' is deprecated but still available. It can be disabled through a
system control register but I can't see a reason why anyone should
disable it especially if 32-bit compatibility is important.


Hmm, right. I tried to read through the whole thread I linked to but
most of it focused around the swp instruction, so I didn't get
complete clarity on how much of an issue this really was wrt setend
- clearly they had picked up on its use and found it worthy of
pointing out, and also more or less suggesting that we should stop
using it - although it hasn't been requested directly so far.

Was the point that the instruction is supposed to be disabled on
vanilla ARMv8 kernels and the android kernels would want to behave
as closely to the vanilla kernels as possible, thus wanting to
change this part in the ARMv8 kernel abi?


The kernel currently does nothing with those system control register
bits (ITD, SED and CP15BEN) but their state after reset is
architecturally unknown (implementation defined). To rely on it either
the kernel or bootloader has to set them.


I see, thanks for clarifying.


Yeah... Disabling if running on = ARMv8 would perhaps make more
sense though, as it is deprecated only there.


The only problem is to determine the architecture version. I fear there
is no cleaner way than parsing AT_PLATFORM from /proc/self/auxv or CPU
architecture from /proc/cpuinfo.


If we care about armv7 instructions which are deprecated on armv8 the
partial deprecation of 'it' is even more annoying. The non-deprecated
'it' on armv8 allows just a single conditional instruction. We use the
deprecated form in 15 files (grep for ' it[te]+ '). Thumb2 is afaik
default on android and ios.


Indeed. This was only touched upon briefly in the discussion, and
IIRC it was concluded that it will have to be supported more
significantly since it's the default generated by compilers.


At least on ios both deprecated 32-bit instructions are not a problem
and I wouldn't expect it to be a problem on android either for the
foreseeable future (as long as both allow installation of 32-bit apps on
armv8 devices).


At least in practice I guess some support for it will be kept in the
android kernels, regardless of what the vanilla kernels do (and I
guess that was the main point of the linked discussion, on
convincing the vanilla kernel to commit to keeping the same
compatibility). Hard to say for sure though so far, given the lack
of such devices.


I think even the vanilla kernel will keep supporting those as long as
the CPU implementation supports it. They won't disable the deprecated
instructions. The discussion revolves around emulating deprecated
instructions in the kernel when they are not supported by the hardware.
I asked for a better way to detect whether deprecatd instructions can be
used.


Ok, thanks for taking over this part. Consider this patch dropped for now.

// Martin
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] arm: Avoid functions using the deprecated 'setend' instruction on modern arches

2014-07-06 Thread Janne Grunau
On 2014-07-04 18:27:54 +0300, Martin Storsjö wrote:
 See [1] for discussion on the issue with using 'setend' on modern
 arm versions.
 
 [1] 
 http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268504.html
 ---
  libavcodec/arm/h264dsp_init_arm.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/libavcodec/arm/h264dsp_init_arm.c 
 b/libavcodec/arm/h264dsp_init_arm.c
 index 92658e7..dd21418 100644
 --- a/libavcodec/arm/h264dsp_init_arm.c
 +++ b/libavcodec/arm/h264dsp_init_arm.c
 @@ -104,8 +104,12 @@ av_cold void ff_h264dsp_init_arm(H264DSPContext *c, 
 const int bit_depth,
  {
  int cpu_flags = av_get_cpu_flags();
  
 -if (have_armv6(cpu_flags))
 +if (have_armv6(cpu_flags)  !(have_vfpv3(cpu_flags) || 
 have_neon(cpu_flags))) {
 +// This function uses the 'setend' instruction which is deprecated
 +// on ARMv8. We don't have cpuflags for ARMv7/ARMv8, but such devices
 +// should have VFPv3 and NEON set.
  c-h264_find_start_code_candidate = 
 ff_h264_find_start_code_candidate_armv6;
 +}

I don't think this is a good solution for the almost non-problem.  
'setend' is deprecated but still available. It can be disabled through a 
system control register but I can't see a reason why anyone should 
disable it especially if 32-bit compatibility is important.

Not that this single optimization is important - it actually annoys me 
since valgrind doesn't emulate setend - but disabling it even on armv7 
seems a little extreme.

If we care about armv7 instructions which are deprecated on armv8 the 
partial deprecation of 'it' is even more annoying. The non-deprecated 
'it' on armv8 allows just a single conditional instruction. We use the 
deprecated form in 15 files (grep for ' it[te]+ '). Thumb2 is afaik 
default on android and ios.

At least on ios both deprecated 32-bit instructions are not a problem 
and I wouldn't expect it to be a problem on android either for the 
foreseeable future (as long as both allow installation of 32-bit apps on 
armv8 devices).

Janne
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] arm: Avoid functions using the deprecated 'setend' instruction on modern arches

2014-07-06 Thread Martin Storsjö

On Sun, 6 Jul 2014, Janne Grunau wrote:


On 2014-07-04 18:27:54 +0300, Martin Storsjö wrote:

See [1] for discussion on the issue with using 'setend' on modern
arm versions.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268504.html
---
 libavcodec/arm/h264dsp_init_arm.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavcodec/arm/h264dsp_init_arm.c 
b/libavcodec/arm/h264dsp_init_arm.c
index 92658e7..dd21418 100644
--- a/libavcodec/arm/h264dsp_init_arm.c
+++ b/libavcodec/arm/h264dsp_init_arm.c
@@ -104,8 +104,12 @@ av_cold void ff_h264dsp_init_arm(H264DSPContext *c, const 
int bit_depth,
 {
 int cpu_flags = av_get_cpu_flags();

-if (have_armv6(cpu_flags))
+if (have_armv6(cpu_flags)  !(have_vfpv3(cpu_flags) || 
have_neon(cpu_flags))) {
+// This function uses the 'setend' instruction which is deprecated
+// on ARMv8. We don't have cpuflags for ARMv7/ARMv8, but such devices
+// should have VFPv3 and NEON set.
 c-h264_find_start_code_candidate = 
ff_h264_find_start_code_candidate_armv6;
+}


I don't think this is a good solution for the almost non-problem.
'setend' is deprecated but still available. It can be disabled through a
system control register but I can't see a reason why anyone should
disable it especially if 32-bit compatibility is important.


Hmm, right. I tried to read through the whole thread I linked to but most 
of it focused around the swp instruction, so I didn't get complete clarity 
on how much of an issue this really was wrt setend - clearly they had 
picked up on its use and found it worthy of pointing out, and also more or 
less suggesting that we should stop using it - although it hasn't been 
requested directly so far.


Was the point that the instruction is supposed to be disabled on vanilla 
ARMv8 kernels and the android kernels would want to behave as closely to 
the vanilla kernels as possible, thus wanting to change this part in the 
ARMv8 kernel abi?



Not that this single optimization is important - it actually annoys me
since valgrind doesn't emulate setend - but disabling it even on armv7
seems a little extreme.


Yeah... Disabling if running on = ARMv8 would perhaps make more sense 
though, as it is deprecated only there.



If we care about armv7 instructions which are deprecated on armv8 the
partial deprecation of 'it' is even more annoying. The non-deprecated
'it' on armv8 allows just a single conditional instruction. We use the
deprecated form in 15 files (grep for ' it[te]+ '). Thumb2 is afaik
default on android and ios.


Indeed. This was only touched upon briefly in the discussion, and IIRC it 
was concluded that it will have to be supported more significantly since 
it's the default generated by compilers.



At least on ios both deprecated 32-bit instructions are not a problem
and I wouldn't expect it to be a problem on android either for the
foreseeable future (as long as both allow installation of 32-bit apps on
armv8 devices).


At least in practice I guess some support for it will be kept in the 
android kernels, regardless of what the vanilla kernels do (and I guess 
that was the main point of the linked discussion, on convincing the 
vanilla kernel to commit to keeping the same compatibility). Hard to say 
for sure though so far, given the lack of such devices.


// Martin
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] arm: Avoid functions using the deprecated 'setend' instruction on modern arches

2014-07-04 Thread Luca Barbato
On 04/07/14 17:27, Martin Storsjö wrote:
 See [1] for discussion on the issue with using 'setend' on modern
 arm versions.
 
 [1] 
 http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268504.html
 ---
  libavcodec/arm/h264dsp_init_arm.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
 

Fine for me.

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel