1025689 - Error: selected processor does not support ARM mode `smulbb' in libopus on Debian armel

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Mike Hommey [:glandium]]

{standard input}: Assembler messages:
{standard input}:528: Error: selected processor does not support ARM mode `smulbb r2,r2,r7'
{standard input}:1128: Error: selected processor does not support ARM mode `smulbb r3,r2,r3'
make[5]: *** [A2NLSF.o] Error 1

Comment 1

[Mike Hommey [:glandium]]

And that boils down to OPUS_ARM_INLINE_EDSP being defined in media/libopus/moz.build unconditionally.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Disable some libopus ARM features on < ARMv6

EDSP is really supported by some ARMv5, but I didn't think it was worth the complication.

Comment 3

[Ralph Giles (:rillian)]

Comment on attachment 8466613 [details] [diff] [review]
Disable some libopus ARM features on < ARMv6

Review of attachment 8466613 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the slow review. I need more information here. What target and toolchain is this? We should use as much asm as we can, especially on slower devices.

::: config/crmf/moz.build
@@ +10,5 @@
>      OS_LIBS += [l for l in CONFIG['NSS_LIBS'].split() if l.startswith('-L')]
>      OS_LIBS += '-lcrmf'
>  else:
>      USE_LIBS += [
> +#TODO : dangerous

This doesn't belong in this patch?

::: media/libopus/moz.build
@@ +28,5 @@
>      DEFINES['OPUS_ARM_ASM'] = True
>      DEFINES['OPUS_ARM_EXTERNAL_ASM'] = True
>      DEFINES['OPUS_ARM_INLINE_ASM'] = True
> +    if int(CONFIG['ARM_ARCH']) >= 6:
> +        DEFINES['OPUS_ARM_INLINE_EDSP'] = True

I thought armv5 was the point of EDSP, but ok.

@@ +31,5 @@
> +    if int(CONFIG['ARM_ARCH']) >= 6:
> +        DEFINES['OPUS_ARM_INLINE_EDSP'] = True
> +        DEFINES['OPUS_ARM_MAY_HAVE_EDSP'] = True
> +        DEFINES['OPUS_ARM_MAY_HAVE_MEDIA'] = True
> +        DEFINES['OPUS_ARM_MAY_HAVE_NEON'] = True

These enable runtime detection code. Do they not compile either?

Comment 4

[Mike Hommey [:glandium]]

(In reply to Ralph Giles (:rillian) from comment #3)
> Comment on attachment 8466613 [details] [diff] [review]
> Disable some libopus ARM features on < ARMv6
> 
> Review of attachment 8466613 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sorry for the slow review. I need more information here. What target and
> toolchain is this? We should use as much asm as we can, especially on slower
> devices.

As per bug subject, this is Debian armel. IOW, armv4t.

> 
> ::: config/crmf/moz.build
> @@ +10,5 @@
> >      OS_LIBS += [l for l in CONFIG['NSS_LIBS'].split() if l.startswith('-L')]
> >      OS_LIBS += '-lcrmf'
> >  else:
> >      USE_LIBS += [
> > +#TODO : dangerous
> 
> This doesn't belong in this patch?

Oops, definitely not.
 
> ::: media/libopus/moz.build
> @@ +28,5 @@
> >      DEFINES['OPUS_ARM_ASM'] = True
> >      DEFINES['OPUS_ARM_EXTERNAL_ASM'] = True
> >      DEFINES['OPUS_ARM_INLINE_ASM'] = True
> > +    if int(CONFIG['ARM_ARCH']) >= 6:
> > +        DEFINES['OPUS_ARM_INLINE_EDSP'] = True
> 
> I thought armv5 was the point of EDSP, but ok.

EDSP is only available on armv5e/armv5te and up, not all armv5.

> @@ +31,5 @@
> > +    if int(CONFIG['ARM_ARCH']) >= 6:
> > +        DEFINES['OPUS_ARM_INLINE_EDSP'] = True
> > +        DEFINES['OPUS_ARM_MAY_HAVE_EDSP'] = True
> > +        DEFINES['OPUS_ARM_MAY_HAVE_MEDIA'] = True
> > +        DEFINES['OPUS_ARM_MAY_HAVE_NEON'] = True
> 
> These enable runtime detection code. Do they not compile either?

I actually didn't test that, but I don't think anyone compiling for < armv6 should run those builds on armv7 considering the gap in generated code. IOW, I don't think it matters whether a build targetting < armv6 has runtime detection and support for media and neon extensions.

Comment 5

[Mike Hommey [:glandium]]

See comment 4

Comment 6

[Ralph Giles (:rillian)]

Sorry, I was waiting for an updated patch. Thanks for the additional context. I guess I'm ok with it as is.

r=me with the spurious change removed.

Comment 7

[Mike Hommey [:glandium]]

Attachment: Disable some libopus ARM features on < ARMv6

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8476329 [details] [diff] [review]
Disable some libopus ARM features on < ARMv6

Per comment 6.

Comment 9

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2db90dd7aa78

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2db90dd7aa78