The cases where we currently pass RUSTFLAGS='-C target_feature=+neon' to rustc when building for 32-bit ARM Android should instead pass --target thumbv7neon-linux-androideabi.

The thumbv7neon-linux-androideabi Rust target enables NEON on the Rust target level, including in the standard library. The packed_simd crate depends on core::arch from the standard library, which needs to be built with NEON enabled for proper performance.

Bug 1521249 is the reason to start using the packed_simd crate at this time.

I don't know if we yet support arm android with a NEON requirement. Certainly the NDK that we compiles with makes NEON an optional thing:

https://developer.android.com/ndk/guides/abis#v7a

So I think that as written, this bug is a WONTFIX?

Our Firefox for Android ARMv7 builds require NEON and have required NEON at least since 53 per bug 1345267.

The default Rust armv7-linux-androideabi target is for the Android baseline documented at https://developer.android.com/ndk/guides/abis#v7a . The thumbv7neon-linux-androideabi target tries to match Firefox's C++ configuration.

The rustflags_neon bits in rules.mk should go away.

Then rust.configure should gain these special cases:

• If MOZ_FPU is neon and Rust target computes to armv7-linux-androideabi, change the Rust target to thumbv7neon-linux-androideabi.
• If MOZ_FPU is neon and Rust target computes to armv7-unknown-linux-gnueabihf, change the Rust target to thumbv7neon-unknown-linux-gnueabihf.

Note that armv7-linux-androideabi is actually a thumb-mode target, despite its name, so the first item just turns on NEON.

For the second item, the change turns on both thumb and NEON compared to armv7-unknown-linux-gnueabihf. It's unclear to me if we enable thumb mode when building C++ code for desktop/glibc ARMv7 Linux, but the NEON-enabled Rust target enables thumb mode in order to make glibc Linux testing as close to Android as possible. (If we don't already enable thumb for C++ for desktop/glibc ARMv7 Linux, we probably should in the interest of making it as close to the Android case as possible.)

Any advice on how the special cases should be implemented in rust.configure?

Considering MOZ_FPU is not available in python configure yet, you're in some chicken-and-egg kind of problem. Incidentally, I started working on moving that stuff to python configure a while ago, with the goal of setting --enable-rust-simd automatically, but never finished. I can come back to that after I'm finished with other build-rust things.

(In reply to Mike Hommey [:glandium] from comment #4)

I can come back to that after I'm finished with other build-rust things.

Thanks.

Both thumbv7neon-linux-androideabi and thumbv7neon-unknown-linux-gnueabihf are now available via rustup on the nightly channel, but the latter narrowly missed the beta train. I commented on the PR to ask about an uplift to beta.

So this wouldn't for before 1.33 or 1.34... assuming --enable-rust-simd is fixed for those versions...

(In reply to Mike Hommey [:glandium] from comment #6)

So this wouldn't for before 1.33 or 1.34... assuming --enable-rust-simd is fixed for those versions...

Right. 1.33 breaks current --enable-rust-simd. On x86, x86_64 and aarch64, it's fixable in 1.33. On ARMv7 the fix requires this change as well so 1.33 on Android and, unless uplifted to Rust beta, 1.34 on glibc Linux. (And, it now seems, some other tweaks that I need to research and didn't expect are needed on ARMv7 in addition to this change.)

(In reply to Henri Sivonen (:hsivonen) from comment #7)

(In reply to Mike Hommey [:glandium] from comment #6)

So this wouldn't for before 1.33 or 1.34... assuming --enable-rust-simd is fixed for those versions...

Right. 1.33 breaks current --enable-rust-simd. On x86, x86_64 and aarch64, it's fixable in 1.33. On ARMv7 the fix requires this change as well so 1.33 on Android and, unless uplifted to Rust beta, 1.34 on glibc Linux. (And, it now seems, some other tweaks that I need to research and didn't expect are needed on ARMv7 in addition to this change.)

Should we just wait for Rust 1.34 because of this? ("fixable" in 1.33 means that we'd have to persuade Rust relman to uplift patches? Or fixable at the crate level?)

My current understanding is this:

• 1.33 breaks simd
• packed_simd seems to work on x86, x86_64 and aarch64 as a replacement (but I should re-run benchmarks to be sure)
• packed_simd without thumbv7neon targets regresses performance relative to simd on ARMv7.
• As of today, unexpectedly, packed_simd also regresses performance with a thumbv7neon target.
• Android thumbv7neon target is in 1.33, but verifying any performance results on Android is hard.
• glibc thumbv7neon, which is the target I actually use for assessing ARMv7 performance, is in 1.34.

It's quite possible that a way to unregress performance on ARMv7 can be discovered on 1.34 and be applied to Android in 1.33 (perhaps by also shipping the stdsimd crate in place of using core::arch in 1.33 if the issue turns out to be an interaction between core::arch and packed_simd--as I understand it, core::arch is a snapshot of stdsimd).

(In reply to Mike Hommey [:glandium] from comment #4)

Considering MOZ_FPU is not available in python configure yet, you're in some chicken-and-egg kind of problem. Incidentally, I started working on moving that stuff to python configure a while ago, with the goal of setting --enable-rust-simd automatically, but never finished. I can come back to that after I'm finished with other build-rust things.

Turns out I need it for my other build-rust things, so it will happen earlier than I thought.

(In reply to Henri Sivonen (:hsivonen) from comment #9)

• As of today, unexpectedly, packed_simd also regresses performance with a thumbv7neon target.

Filed as https://github.com/rust-lang-nursery/packed_simd/issues/215

Indeed, it looks like due to https://github.com/rust-lang-nursery/packed_simd/issues/216 for now it's necessary to make packed_simd use core_arch from crates.io instead of core::arch from the standard library, so this bug is likely moot until packed_simd becomes able to use core::arch on the thumbv7neon targets.

(In reply to Henri Sivonen (:hsivonen) from comment #12)

Indeed, it looks like due to https://github.com/rust-lang-nursery/packed_simd/issues/216 for now it's necessary to make packed_simd use core_arch from crates.io instead of core::arch from the standard library, so this bug is likely moot until packed_simd becomes able to use core::arch on the thumbv7neon targets.

This has now been fixed in packed_simd.

(In reply to Mike Hommey [:glandium] from comment #10)

Turns out I need it for my other build-rust things, so it will happen earlier than I thought.

Regarding core::arch vs. core_arch: I tried vendoring core_arch into m-c, and it didn't build on first attempt.

Should I expect this bug to be fixable by the time Rust 1.33 is released? Or should I investigate how to make vendoring core_arch work?

I'd much prefer using core::arch, which requires a fix for this bug for 32-bit ARM, because if we vendor core_arch, we'd need to track its breakage relative to compiler changes, whereas core::arch is guaranteed to match the compiler.

(In reply to Nathan Froyd [:froydnj] from comment #8)

Should we just wait for Rust 1.34 because of this?

thumbv7neon-unknown-linux-gnueabihf got uplifted to 1.33 and it seems that the fix to use less RAM when building packed_simd is in the process of being uplifted to 1.33.

I can't guarantee something will be ready when 1.33 is released, but it shouldn't be too far off.

That said, before that, we should make --enable-rust-simd error out if building with rust >= 1.33, and uplift that to all branches, including esr, because wasting all this time to fail during the build is not going to help anybody. Would you take care of that?

(In reply to Mike Hommey [:glandium] from comment #15)

That said, before that, we should make --enable-rust-simd error out if building with rust >= 1.33, and uplift that to all branches, including esr, because wasting all this time to fail during the build is not going to help anybody. Would you take care of that?

Was this done?

(In reply to Mike Hommey [:glandium] from comment #16)

(In reply to Mike Hommey [:glandium] from comment #15)

That said, before that, we should make --enable-rust-simd error out if building with rust >= 1.33, and uplift that to all branches, including esr, because wasting all this time to fail during the build is not going to help anybody. Would you take care of that?

Was this done?

Not yet.

(In reply to Mike Hommey [:glandium] from comment #16)

(In reply to Mike Hommey [:glandium] from comment #15)

That said, before that, we should make --enable-rust-simd error out if building with rust >= 1.33, and uplift that to all branches, including esr, because wasting all this time to fail during the build is not going to help anybody. Would you take care of that?

Was this done?

Bug 1531655

(In reply to Mike Hommey [:glandium] from comment #4)

Considering MOZ_FPU is not available in python configure yet, you're in some chicken-and-egg kind of problem.

Do we actually need to keep build system support for building for Android with a non-neon MOZ_FPU value? That is, instead of

• If MOZ_FPU is neon and Rust target computes to armv7-linux-androideabi, change the Rust target to thumbv7neon-linux-androideabi.

could we unconditionally replace armv7-linux-androideabi with thumbv7neon-linux-androideabi?

(Won't work for the glibc desktop case for distro purposes, of course.)

Do we have an estimate for when this will land?

ni glandium for questions in comment 20 and comment 19.

I'm half-way through a patch for bug 1524429. It should be up for review today or tomorrow.

Depends on D24323

Newer versions of rust come with a specialized arm target that matches
more closely our armv7 targets (with neon and thumb2), so use that when
possible.

Depends on D24324

https://hg.mozilla.org/mozilla-central/rev/f3cbeb24bd86
https://hg.mozilla.org/mozilla-central/rev/ca1eb7e8ff60
https://hg.mozilla.org/mozilla-central/rev/a01996588f99

Looks like stars don't align sometimes even if the thumv7neon target is selected:

 0:01.10 error: failed to run custom build command for `target-lexicon v0.2.0`
 0:01.10 process didn't exit successfully: `/home/fabrice/dev/gecko-dev/obj-arm-unknown-linux-androideabi/release/build/target-lexicon-42376ff18ae281f1/build-script-build` (exit code: 101)
 0:01.10 --- stderr
 0:01.10 thread 'main' panicked at 'can't find custom target thumbv7neon-linux-androideabi', /home/fabrice/dev/gecko-dev/third_party/rust/target-lexicon/build.rs:99:5
 0:01.10 stack backtrace:
 0:01.10    0:     0x56496ab24773 - std::sys::unix::backtrace::tracing::imp::unwind_backtrace::hf8722b0178fb1b63
 0:01.10                                at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
 0:01.10    1:     0x56496ab20508 - std::sys_common::backtrace::_print::hc40139e5f1d656ee
 0:01.10                                at src/libstd/sys_common/backtrace.rs:70
 0:01.10    2:     0x56496ab23542 - std::panicking::default_hook::{{closure}}::h993d43465919c16a
 0:01.10                                at src/libstd/sys_common/backtrace.rs:58
 0:01.11                                at src/libstd/panicking.rs:200
 0:01.11    3:     0x56496ab232b4 - std::panicking::default_hook::h73d2c2ec3d9ba5a4
 0:01.11                                at src/libstd/panicking.rs:215
 0:01.11    4:     0x56496ab23ba0 - std::panicking::rust_panic_with_hook::h744417edfe714d72
 0:01.11                                at src/libstd/panicking.rs:478
 0:01.11    5:     0x56496ab23721 - std::panicking::continue_panic_fmt::h3557b3c3fa21b47b
 0:01.11                                at src/libstd/panicking.rs:385
 0:01.11    6:     0x56496ab2366e - std::panicking::begin_panic_fmt::h376894437226fc7c
 0:01.11                                at src/libstd/panicking.rs:340
 0:01.11    7:     0x56496ab167e6 - build_script_build::main::h455eae2fc129055f
 0:01.11    8:     0x56496ab11b12 - std::rt::lang_start::{{closure}}::hf25e5eadc14412a7
 0:01.11    9:     0x56496ab235f2 - std::panicking::try::do_call::h7a0381557c6c2cee
 0:01.11                                at src/libstd/rt.rs:49
 0:01.11                                at src/libstd/panicking.rs:297
 0:01.11   10:     0x56496ab25e89 - __rust_maybe_catch_panic
 0:01.11                                at src/libpanic_unwind/lib.rs:92
 0:01.11   11:     0x56496ab240f5 - std::rt::lang_start_internal::he0d8d06abc6f912f
 0:01.11                                at src/libstd/panicking.rs:276
 0:01.11                                at src/libstd/panic.rs:388
 0:01.11                                at src/libstd/rt.rs:48
 0:01.11   12:     0x56496ab18411 - main
 0:01.11   13:     0x7faf6d48a09a - __libc_start_main
 0:01.11   14:     0x56496ab0f179 - _start
 0:01.11   15:                0x0 - <unknown>

Exactly! It is found during mach configure, but not here.

Oh, I should mention. This is under Linux fedora27 using standard build tools as intalled by mach bootstrap.

(In reply to mac198442 from comment #29)

Exactly! It is found during mach configure, but not here.

I guess I should have said there. mach bootstrap works but the build fails in target-lexicon
target-lexicontarget-lexicontarget-lexicon

(In reply to mac198442 from comment #31)

(In reply to mac198442 from comment #29)

Exactly! It is found during mach configure, but not here.

I guess I should have said there. mach bootstrap works but the build fails
in target-lexicon
target-lexicontarget-lexicontarget-lexicon

i hve no idea what happened tried to say fails in target-lexicon.