[FFmpeg-devel] [PATCH] arm: only enable setend on ARMv6

[Michael Niedermayer]

On Fri, Jun 05, 2015 at 01:40:31PM +0200, Andreas Cadhalpun wrote:
> On 05.06.2015 00:47, Michael Niedermayer wrote:
> > On Thu, Jun 04, 2015 at 11:17:45PM +0200, Andreas Cadhalpun wrote:
> >> Without this check it causes SIGILL crashes on ARMv5.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavutil/arm/cpu.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavutil/arm/cpu.c b/libavutil/arm/cpu.c
> >> index f1683e8..ac42a05 100644
> >> --- a/libavutil/arm/cpu.c
> >> +++ b/libavutil/arm/cpu.c
> >> @@ -128,7 +128,7 @@ int ff_get_cpu_flags_arm(void)
> >>         trickle down. */
> >>      if (flags & (AV_CPU_FLAG_VFPV3 | AV_CPU_FLAG_NEON))
> >>          flags |= AV_CPU_FLAG_ARMV6T2;
> >> -    else
> >> +    else if (flags & AV_CPU_FLAG_ARMV6)
> >>      /* Some functions use the 'setend' instruction which is deprecated on ARMv8
> >>       * and serializing on some ARMv7 cores. This ensures such functions
> >>       * are only enabled on ARMv6. */
> > 
> > shouldnt this be after
> > "- if (flags & AV_CPU_FLAG_ARMV6T2)
> >         flags |= AV_CPU_FLAG_ARMV6;
> > "
> 
> That would require duplicating the !(flags & (AV_CPU_FLAG_VFPV3 | AV_CPU_FLAG_NEON)
> check.
> 
> > or (AV_CPU_FLAG_ARMV6T2 | AV_CPU_FLAG_ARMV6)
> 
> That seems better, assuming setend is useful on ARMv6t2.

AFAIK its even useful on some later cpus that have NEON (that is
IIRC the one i tested long ago)


> Updated patch attached.

LGTM

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150605/0a6d0461/attachment.asc>