Closed
Bug 1434589
Opened 6 years ago
Closed 6 years ago
Compilation on Aarch64 fail in webrtc: g++: error: unrecognized command line option ‘-msse2
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox-esr60 wontfix, firefox63 fixed)
RESOLVED
FIXED
mozilla63
People
(Reporter: bruno, Assigned: m_kato)
References
Details
Attachments
(2 files, 2 obsolete files)
User Agent: Mozilla/5.0 (X11; Linux aarch64; rv:59.0) Gecko/20100101 Firefox/59.0 Build ID: 20180119195441 Steps to reproduce: Compilation of FF on my NVIDIA Jetson-TX1 (Aarch64) was ok with b2cb61e83ac50115a28f04aaa8a32d4db90aad23.zstd-max.hg repo. I've updated the repo this morning and now i've got this error: g++: error: unrecognized command line option ‘-msse2’. There's no SSE on ARM ;) Actual results: o_engine_gn/Unified_cpp_video_engine_gn0.cpp:2:0: 151:30.65 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/video_engine/desktop_capture_impl.cc:435:1: warning: when initialized here [-Wreorder] 151:30.65 DesktopCaptureImpl::DesktopCaptureImpl(const int32_t id) 151:30.65 ^ 151:32.24 libvideo_engine_gn.a.desc 151:36.28 libprimitives_gn.a.desc 151:38.28 In file included from /home/bruno/usb/src/mozilla-central/obj-aarch64-unknown-linux-gnu/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_capture_gn/Unified_cpp_desktop_capture_gn0.cpp:65:0: 151:38.28 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_device_info.cc: In member function ‘virtual int32_t webrtc::DesktopDeviceInfoImpl::getDesktopDisplayDeviceInfo(int32_t, webrtc::DesktopDisplayDevice&)’: 151:38.29 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_device_info.cc:186:27: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] 151:38.29 if(nIndex < 0 || nIndex >= desktop_display_list_.size()) { 151:38.29 ^ 151:38.29 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_device_info.cc: In member function ‘virtual int32_t webrtc::DesktopDeviceInfoImpl::getWindowInfo(int32_t, webrtc::DesktopDisplayDevice&)’: 151:38.29 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_device_info.cc:205:28: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] 151:38.29 if (nIndex < 0 || nIndex >= desktop_window_list_.size()) { 151:38.29 ^ 151:38.29 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_device_info.cc: In member function ‘virtual int32_t webrtc::DesktopDeviceInfoImpl::getApplicationInfo(int32_t, webrtc::DesktopApplication&)’: 151:38.29 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_device_info.cc:226:27: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] 151:38.29 if(nIndex < 0 || nIndex >= desktop_application_list_.size()) { 151:38.29 ^ 151:38.45 In file included from /home/bruno/usb/src/mozilla-central/obj-aarch64-unknown-linux-gnu/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_capture_gn/Unified_cpp_desktop_capture_gn0.cpp:137:0: 151:38.45 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/modules/desktop_capture/x11/desktop_device_info_x11.cc: In member function ‘virtual void webrtc::DesktopDeviceInfoX11::InitializeApplicationList()’: 151:38.45 /home/bruno/usb/src/mozilla-central/media/webrtc/trunk/webrtc/modules/desktop_capture/x11/desktop_device_info_x11.cc:83:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] 151:38.45 if (processId == getpid()) { 151:38.45 ^ 151:46.03 libdesktop_capture_gn.a.desc 151:46.23 g++: error: unrecognized command line option ‘-msse2’ 151:46.23 /home/bruno/usb/src/mozilla-central/config/rules.mk:1047 : la recette pour la cible « Unified_cpp_common_audio_sse2_gn0.o » a échouée 151:46.23 make[4]: *** [Unified_cpp_common_audio_sse2_gn0.o] Erreur 1 151:46.23 /home/bruno/usb/src/mozilla-central/config/recurse.mk:73 : la recette pour la cible « media/webrtc/trunk/webrtc/common_audio/common_audio_sse2_gn/target » a échouée 151:46.23 make[3]: *** [media/webrtc/trunk/webrtc/common_audio/common_audio_sse2_gn/target] Erreur 2 151:46.23 /home/bruno/usb/src/mozilla-central/config/recurse.mk:32 : la recette pour la cible « compile » a échouée 151:46.23 make[2]: *** [compile] Erreur 2 151:46.23 /home/bruno/usb/src/mozilla-central/config/rules.mk:434 : la recette pour la cible « default » a échouée 151:46.23 make[1]: *** [default] Erreur 2 151:46.23 client.mk:168 : la recette pour la cible « build » a échouée 151:46.23 make: *** [build] Erreur 2 151:46.27 0 compiler warnings present. 151:46.98 /usr/bin/notify-send --app-name=Mozilla Build System Mozilla Build System Build failed
Assignee | ||
Comment 1•6 years ago
|
||
Same issue of bug 1433725. GN doesn't generate aarch64 JSON file since it isn't tier-1.
Component: Untriaged → Build Config
Product: Firefox → Core
Hardware: Unspecified → ARM64
See Also: → 1433725
Comment 2•6 years ago
|
||
You mean bug 1430094. Bug 1433725 is different, although after bug 1393119, it might end up being the same thing.
Updated•6 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•6 years ago
|
||
Bug 1436513 is about this problem on ARMv7.
Updated•6 years ago
|
Product: Core → Firefox Build System
Comment 5•6 years ago
|
||
I am seeing this issue as well. I am attempting to cross compile for Raspberry Pi. Bug 1450742 > This is because we've switched to using gn to generate moz.build files. We don't currently have any gn configs for Linux on ARM, so the generated files attempt to use -msse2 for everything. Bug 1436513 I am unfamiliar with this part of the build system. Is there someone who can guide me to add gn configs for Linux on ARM? I think it is a good idea to get Firefox vanilla running on ARM OS's like Raspbian especially since Iceweasel is no longer available for newer builds of Mozilla ESR.
Comment 6•6 years ago
|
||
There is https://firefox-source-docs.mozilla.org/build/buildsystem/gn.html but this helps not that much, at least for me :) . So far I understand this there are more profiles needed in mozilla/media/webrtc/gn-configs for the other architectures.
Comment 7•6 years ago
|
||
I made more progress in the build. See bug 1450742
Updated•6 years ago
|
status-firefox-esr60:
--- → affected
Comment 8•6 years ago
|
||
The problem is Mozilla's own build script in media/webrtc/trunk/moz.build which unconditionally builds the SSE2 versions of the video and audio components of WebRTC on Linux. Interestingly, this does not affect FreeBSD which has the proper conditionals for ARM CPUs with NEON and x86 CPUs. I was able to build Firefox from Mercurial with this fixed just fine on ppc64 big-endian. I'll attach a patch shortly. @Carsten: You can also use this patch for Debian's Thunderbird package.
Comment 9•6 years ago
|
||
Attachment #8980946 -
Flags: review?(rjesup)
Attachment #8980946 -
Flags: review?(mh+mozilla)
Attachment #8980946 -
Flags: review?(martin)
Attachment #8980946 -
Flags: review?(c.schoenert)
Comment 10•6 years ago
|
||
I have attached a patch now which updates the moz.build to use the SSE2 stuff on x86_64 CPUs now. However, this particular moz.build script is apparently generated by a GN build file. However, I couldn't find the original GN file from which the moz.build is generated. Someone more familiar with the GN build system should be able to patch the GN file though. If I find it in the meantime, I'll send out a revised patch.
Comment 11•6 years ago
|
||
Comment on attachment 8980946 [details] [diff] [review] 1434589_media_webrtc_Fix_conditional_compilation_of_CPU-specific_code.patch Review of attachment 8980946 [details] [diff] [review]: ----------------------------------------------------------------- The following is written at the beginning of that file: ### This moz.build was AUTOMATICALLY GENERATED from a GN config, ### ### DO NOT edit it by hand. ###
Attachment #8980946 -
Flags: review?(rjesup)
Attachment #8980946 -
Flags: review?(mh+mozilla)
Attachment #8980946 -
Flags: review?(martin)
Attachment #8980946 -
Flags: review?(c.schoenert)
Attachment #8980946 -
Flags: review-
Comment 12•6 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #11) > Comment on attachment 8980946 [details] [diff] [review] > 1434589_media_webrtc_Fix_conditional_compilation_of_CPU-specific_code.patch > > Review of attachment 8980946 [details] [diff] [review]: > ----------------------------------------------------------------- > > The following is written at the beginning of that file: > > ### This moz.build was AUTOMATICALLY GENERATED from a GN config, ### > ### DO NOT edit it by hand. ### Yes, as I said, I couldn't find the GN file yet from which this is generated. The patch is just meant as a first heads-up, in the case someone is able to find the GN file before me.
Comment hidden (obsolete) |
Comment hidden (obsolete) |
Comment 15•6 years ago
|
||
(In reply to John Paul Adrian Glaubitz from comment #8) > Interestingly, this does not affect FreeBSD which has the proper conditionals > for ARM CPUs with NEON and x86 CPUs. GnMozbuildWriter generates moz.build files based on all configs. It doesn't know how to distinguish which files are OS-specific and which are CPU-specific. SSE2/NEON are gated by target_cpu, so simply adding more configs extends conditionals. https://reviewboard-hg.mozilla.org/gecko/diff/dc8090557539/media/webrtc/trunk/moz.build https://searchfox.org/mozilla-central/rev/b47af5169898/media/webrtc/trunk/webrtc/build/webrtc.gni#113 If you have Linux armv7/aarch64/powerpc*/etc chroot then just "./mach configure && ./mach build-backend -b GnConfigGen" like the docs say (ditto for --enable-debug). Otherwise, copy-paste existing configs and adjust them until "./mach build-backend -b GnMozbuildWriter" generates the desired output in moz.build files.
Comment 16•6 years ago
|
||
I'm hitting this too for ppc64le, but I can't find a GnMozbuildWriter to regenerate moz.build. Where is this located? I added a ppc64le configuration to build/toolchain/linux/BUILD.gn but -msse2 is still sneaking in, and ./mach build-backend doesn't seem to regenerate anything.
Comment 17•6 years ago
|
||
NVM, I think I have it sorted. Patch for ppc64 coming up. For the record: compiling gn *sucks* and there has got to be a better solution than depending on that steaming heap.
Comment 18•6 years ago
|
||
This includes a GN config from this ppc64le Linux system which, after regeneration of moz.build, excludes SSE2 from non-x86 builds on Linux. I didn't make any additional changes to moz.build other than to rerun the steps in https://firefox-source-docs.mozilla.org/build/buildsystem/gn.html , but it should fix everyone else on Linux with this issue as a nice side effect. Mike, if you are not the correct reviewer, please advise.
Assignee: nobody → spectre
Attachment #8980946 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8981056 -
Flags: review?(mh+mozilla)
Comment 19•6 years ago
|
||
Comment on attachment 8981056 [details] [diff] [review] Add ppc64 to webrtc, regenerate moz.build to remove SSE2 dependency for Linux Review of attachment 8981056 [details] [diff] [review]: ----------------------------------------------------------------- Redirecting. Note the bug is about aarch64, and this patch adds support for ppc64, so technically speaking, this doesn't fix the bug as filed.
Attachment #8981056 -
Flags: review?(mh+mozilla) → review?(core-build-config-reviews)
Comment 20•6 years ago
|
||
No disagreement, but it should allow those systems to build as well as a side effect.
Comment 21•6 years ago
|
||
(In reply to Cameron Kaiser [:spectre] from comment #20) > No disagreement, but it should allow those systems to build as well as a > side effect. I'm puzzled. That suggests you edited moz.build manually rather than regenerate it.
Comment 22•6 years ago
|
||
No, I absolutely didn't. That was exactly following the instructions given. The moz.build it generated now checks for x86 and x86_64 first. I thought it was odd as well, but I swear I didn't do that myself.
Comment 23•6 years ago
|
||
(In reply to Cameron Kaiser [:spectre] from comment #18) > Created attachment 8981056 [details] [diff] [review] > Add ppc64 to webrtc, regenerate moz.build to remove SSE2 dependency for > Linux A 6000-line patch to add an additional compile target to a build system? What? Did you write that file manually or did you create it? And, I agree, building GN from source sucks. So far, I was unable to build it myself as it requires a working GN binary if I remember correctly which exists for x86 only. Can you give a quick summary how to add additional platforms so I can add more of the targets we have in Debian?
Comment 24•6 years ago
|
||
The file was generated. The basic notion was I started with https://gist.github.com/mohamed/4fa7eb75807463d4dfa3 , picked up the additional deps from https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=gn-git&id=1ad9044b1504c019779cb3da4f4d10e7a3d39377 and put them in buildtools/third_party/{libc++,libc++abi}/trunk , hacked tools/gn/bootstrap/bootstrap.py to ignore these files # 'base/trace_event/heap_profiler_heap_dump_writer.cc', # 'base/trace_event/heap_profiler_serialization_state.cc', # 'base/trace_event/heap_profiler_stack_frame_deduplicator.cc', # 'base/trace_event/heap_profiler_type_name_deduplicator.cc', that are apparently not in tree anymore, because Google can't keep their own code in sync, and then ( cd tools/gn && ./bootstrap/bootstrap.py -s ) to build. Then a tiny tweak to mach because it ignored my $GN variable, and away we go. Just like falling off a log into a lake full of alligators swimming in muriatic acid in hell.
Comment 25•6 years ago
|
||
Comment on attachment 8981056 [details] [diff] [review] Add ppc64 to webrtc, regenerate moz.build to remove SSE2 dependency for Linux Review of attachment 8981056 [details] [diff] [review]: ----------------------------------------------------------------- Please file a new bug for this, since as glandium pointed out, this is not AArch64-related. ::: media/webrtc/trunk/moz.build @@ +181,4 @@ > "/media/webrtc/trunk/webrtc/modules/video_processing/video_processing_neon_gn" > ] > > +if CONFIG["CPU_ARCH"] == "x86" and CONFIG["OS_TARGET"] == "Linux": Why not add these directories under the `CONFIG["OS_TARGET"] == "Linux"` and say: if CONFIG["CPU_ARCH"] in ("x86", "x86-64"): DIRS += [ ...sse2 directories... ] ? That seems better than duplicating things.
Attachment #8981056 -
Flags: review?(core-build-config-reviews)
Comment 26•6 years ago
|
||
I'll file the new bug, but clarification requested: I thought the requirement was that we *don't* modify moz.build, just let the build system regenerate it. That's what I did here -- what I posted in the patch was the diff from the change GnMozbuildWriter created from the new config I generated. If it's preferred for me to make a "better" change to moz.build manually, I can do that too (if so, the change I make will also fix this one, though I'll still file a separate bug as requested). Please clarify which you would want.
Flags: needinfo?(nfroyd)
Comment 27•6 years ago
|
||
(In reply to Cameron Kaiser [:spectre] from comment #26) > I'll file the new bug, but clarification requested: I thought the > requirement was that we *don't* modify moz.build, just let the build system > regenerate it. That's what I did here -- what I posted in the patch was the > diff from the change GnMozbuildWriter created from the new config I > generated. If it's preferred for me to make a "better" change to moz.build > manually, I can do that too (if so, the change I make will also fix this > one, though I'll still file a separate bug as requested). Please clarify > which you would want. My fault! I didn't look closely enough to realize you were changing an autogenerated moz.build. I guess we will live with it; please just file the new bug with the patch, and I'll stamp r+ there. Thank you!
Flags: needinfo?(nfroyd)
Comment 28•6 years ago
|
||
Done (bug 1465274). Thanks!
Comment hidden (obsolete) |
Comment 30•6 years ago
|
||
Shouldn't we also add "if CONFIG["CPU_ARCH"] == "aarch64" and CONFIG["OS_TARGET"] == "Linux":" to enable the Neon code on Linux/arm64?
Comment 31•6 years ago
|
||
(In reply to Cameron Kaiser [:spectre] from comment #17) > NVM, I think I have it sorted. Patch for ppc64 coming up. For the record: > compiling gn *sucks* and there has got to be a better solution than > depending on that steaming heap. We're open to ideas. We know it's a PITA but with Google switching projects like WebRTC to use gn as their build systems we are not left with many good options. :(
Comment 32•6 years ago
|
||
(In reply to John Paul Adrian Glaubitz from comment #30) > Shouldn't we also add "if CONFIG["CPU_ARCH"] == "aarch64" and > CONFIG["OS_TARGET"] == "Linux":" to enable the Neon code on Linux/arm64? If you create a config for aarch64 with the same process, then AFAIK this should "just happen" when the moz.build is regenerated.
Comment 33•6 years ago
|
||
(unassigning myself since bug 1465274 is fixed)
Attachment #8981056 -
Attachment is obsolete: true
Assignee | ||
Comment 34•6 years ago
|
||
I need this due to my aarch64 device, so patch is coming soon.
Assignee: nobody → m_kato
Assignee | ||
Comment 35•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fe2b92fe67f06d5fafa95a3d88b50c69ac5744f
Comment 36•6 years ago
|
||
(In reply to Makoto Kato [:m_kato] from comment #35) > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=2fe2b92fe67f06d5fafa95a3d88b50c69ac5744f That probably won't be necessary after bug 1467319
Assignee | ||
Comment 37•6 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #36) > (In reply to Makoto Kato [:m_kato] from comment #35) > > https://treeherder.mozilla.org/#/ > > jobs?repo=try&revision=2fe2b92fe67f06d5fafa95a3d88b50c69ac5744f > > That probably won't be necessary after bug 1467319 Nice!
Assignee | ||
Comment 38•6 years ago
|
||
When building binaries for Linux/aarch64, ./mach configure cannot generate Makefile by the following error. So we need Linux/aarch64's gn-configs. 1:07.80 mozbuild.frontend.reader.SandboxValidationError: 1:07.80 ============================== 1:07.80 FATAL ERROR PROCESSING MOZBUILD FILE 1:07.80 ============================== 1:07.80 The error occurred while processing the following file or one of the files it includes: 1:07.80 /hg/mozilla-central/media/webrtc/trunk/webrtc/common_audio/common_audio_c_gn/moz.build 1:07.80 The error occurred when validating the result of the execution. The reported error is: 1:07.80 Source file should only be added to UNIFIED_SOURCES once: /media/webrtc/trunk/webrtc/common_audio/signal_processing/complex_bit_reverse.c
Assignee | ||
Comment 39•6 years ago
|
||
Comment 40•6 years ago
|
||
Comment on attachment 9001934 [details] Bug 1434589 - Part 2. Update moz.build. r?Build Chris Manchester (:chmanchester) has approved the revision.
Attachment #9001934 -
Flags: review+
Comment 41•6 years ago
|
||
Comment on attachment 9001933 [details] Bug 1434589 - Part 1. Add gn-configs for Linux/aarch64. r?Build Chris Manchester (:chmanchester) has approved the revision.
Attachment #9001933 -
Flags: review+
Comment 42•6 years ago
|
||
Pushed by m_kato@ga2.so-net.ne.jp: https://hg.mozilla.org/integration/autoland/rev/e09c5fef9d0d Part 1. Add gn-configs for Linux/aarch64. r=chmanchester
Comment 43•6 years ago
|
||
Pushed by m_kato@ga2.so-net.ne.jp: https://hg.mozilla.org/integration/autoland/rev/ea96184d8874 Part 2. Update moz.build. r=chmanchester
Comment 44•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e09c5fef9d0d https://hg.mozilla.org/mozilla-central/rev/ea96184d8874
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment 46•6 years ago
|
||
Could you please backport this fix on firefox 60 esr? thanks
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•