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)

59 Branch
ARM64
Unspecified
defect
Not set
normal

Tracking

(firefox-esr60 wontfix, firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr60 --- wontfix
firefox63 --- fixed

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
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
You mean bug 1430094. Bug 1433725 is different, although after bug 1393119, it might end up being the same thing.
Blocks: 1393119
See Also: 14337251430094
Status: UNCONFIRMED → NEW
Ever confirmed: true
Bug 1436513 is about this problem on ARMv7.
Product: Core → Firefox Build System
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.
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.
I made more progress in the build. See bug 1450742
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.
Attachment #8980946 - Flags: review?(rjesup)
Attachment #8980946 - Flags: review?(mh+mozilla)
Attachment #8980946 - Flags: review?(martin)
Attachment #8980946 - Flags: review?(c.schoenert)
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 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-
(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.
(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.
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.
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.
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 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)
No disagreement, but it should allow those systems to build as well as a side effect.
(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.
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.
(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?
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 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)
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)
(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)
Done (bug 1465274). Thanks!
Shouldn't we also add "if CONFIG["CPU_ARCH"] == "aarch64" and CONFIG["OS_TARGET"] == "Linux":" to enable the Neon code on Linux/arm64?
(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. :(
(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.
(unassigning myself since bug 1465274 is fixed)
Assignee: spectre → nobody
Status: ASSIGNED → NEW
See Also: → 1465274
Attachment #8981056 - Attachment is obsolete: true
I need this due to my aarch64 device, so patch is coming soon.
Assignee: nobody → m_kato
(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
(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!
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
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 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+
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
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/ea96184d8874
Part 2. Update moz.build. r=chmanchester
https://hg.mozilla.org/mozilla-central/rev/e09c5fef9d0d
https://hg.mozilla.org/mozilla-central/rev/ea96184d8874
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Could you please backport this fix on firefox 60 esr? thanks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: