Add NASM to our build system

RESOLVED FIXED in Firefox 65

Status

enhancement
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: achronop, Assigned: TD-Linux)

Tracking

unspecified
mozilla65
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(6 attachments, 1 obsolete attachment)

Reporter

Description

8 months ago
dav1d upstream uses NASM and will simplify the import of the library.
Reporter

Comment 1

8 months ago
yasm upstream is dead and we should switch to nasm for avx-512 support
Reporter

Comment 2

8 months ago
yasm vs nasm errors:

[achronop@fedora firefox]$ ./mach build
 0:00.24 Clobber not needed.
 0:00.24 Adding make options from /home/achronop/myrepos/mozilla/firefox/mozconfig
    MOZ_OBJDIR=/home/achronop/myrepos/mozilla/firefox/obj-x86_64-pc-linux-gnu
    OBJDIR=/home/achronop/myrepos/mozilla/firefox/obj-x86_64-pc-linux-gnu
    FOUND_MOZCONFIG=/home/achronop/myrepos/mozilla/firefox/mozconfig
    export FOUND_MOZCONFIG
 0:00.24 /usr/bin/gmake -f client.mk -s
 0:00.29 Build configuration changed. Regenerating backend.
 0:00.38 Reticulating splines...
 0:01.62  0:01.24 File already read. Skipping: /home/achronop/myrepos/mozilla/firefox/gfx/angle/targets/angle_common/moz.build
 0:08.65 Finished reading 1925 moz.build files in 2.03s
 0:08.65 Read 65 gyp files in parallel contributing 0.00s to total wall time
 0:08.65 Processed into 9787 build config descriptors in 2.71s
 0:08.65 RecursiveMake backend executed in 2.97s
 0:08.65   3487 total backend files; 0 created; 1 updated; 3486 unchanged; 0 deleted; 34 -> 1282 Makefile
 0:08.65 FasterMake backend executed in 0.23s
 0:08.65   11 total backend files; 0 created; 0 updated; 11 unchanged; 0 deleted
 0:08.65 Total wall time: 8.28s; CPU time: 8.26s; Efficiency: 100%; Untracked: 0.33s
 0:09.01 Elapsed: 0.01s; From dist/xpi-stage: Kept 97 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:09.02 Elapsed: 0.00s; From dist/public: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:09.04 Elapsed: 0.00s; From dist/private: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:09.11 Elapsed: 0.08s; From _tests: Kept 1098 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:09.17 Elapsed: 0.16s; From dist/bin: Kept 2458 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:09.29 Elapsed: 0.25s; From dist/include: Kept 5509 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:09.40 media/libdav1d/src/x86/10bd_ipred_init.c.stub
 0:09.47 media/libdav1d/src/x86/10bd_itx_init.c.stub
 0:09.53 media/libdav1d/src/x86/10bd_loopfilter_init.c.stub
 0:09.59 media/libdav1d/src/x86/10bd_mc_init.c.stub
 0:09.65 media/libdav1d/src/x86/8bd_ipred_init.c.stub
 0:09.71 media/libdav1d/src/x86/8bd_itx_init.c.stub
 0:09.77 media/libdav1d/src/x86/8bd_loopfilter_init.c.stub
 0:09.83 media/libdav1d/src/x86/8bd_mc_init.c.stub
 0:09.95 js/src/rust/force-cargo-library-build
 0:10.02 media/libdav1d/src/x86
 0:10.16 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:105: error: invalid size for operand 2
 0:10.16 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:110: error: invalid size for operand 2
 0:10.16 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:124: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:129: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:163: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:164: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:368: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:381: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:408: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:467: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:600: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:760: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:851: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:874: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:876: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:895: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:897: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:899: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:901: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:972: error: invalid size for operand 2
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:1125: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:1127: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:1166: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:1168: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:1170: error: invalid number of operands
 0:10.17 /home/achronop/myrepos/mozilla/firefox/third_party/dav1d/src/x86/ipred.asm:1172: error: invalid number of operands
 0:10.17 gmake[4]: *** [/home/achronop/myrepos/mozilla/firefox/config/rules.mk:807: ipred.o] Error 1
 0:10.17 gmake[4]: *** Waiting for unfinished jobs....
 0:10.18     Finished dev [optimized + debuginfo] target(s) in 0.22s
 0:10.20 toolkit/library/rust/force-cargo-library-build
 0:10.61     Finished dev [optimized + debuginfo] target(s) in 0.40s
 0:11.56 gmake[3]: *** [/home/achronop/myrepos/mozilla/firefox/config/recurse.mk:74: media/libdav1d/src/x86/target] Error 2
 0:11.56 gmake[3]: *** Waiting for unfinished jobs....
 0:11.59 gmake[2]: *** [/home/achronop/myrepos/mozilla/firefox/config/recurse.mk:34: compile] Error 2
 0:11.59 gmake[1]: *** [/home/achronop/myrepos/mozilla/firefox/config/rules.mk:431: default] Error 2
 0:11.59 gmake: *** [client.mk:125: build] Error 2
 0:11.61 302 compiler warnings present.
 0:11.65 ccache (direct) hit rate: 100.0%; (preprocessed) hit rate: 0.0%; miss rate: 0.0%
This is a pain. :-/ It looks like Chromium has had this same conversation.

In terms of compatibility we'd need to test the various libraries in the tree that currently use yasm:
https://dxr.mozilla.org/mozilla-central/search?q=USE_YASM+path%3Abuild&redirect=true

The biggest ones there are ffvpx, libaom and libjpeg-turbo (the config/external/icu/data bit is a trivial bit of assembler).

If we need to require nasm for the build we'd want to do the following things:
1. Add toolchain jobs to build nasm binaries in CI. We've got a bunch of examples to work from: https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/toolchain/linux.yml . This both lets us require nasm for our builds in CI and also gives us a way to provide prebuilt binaries for developers.
2. Add support for installing nasm to `mach bootstrap`. If we've got toolchain tasks then the easy route is to just install those binaries like we do a few other tools.
3. Inform developers of the new dependency (posting to dev-platform etc).
4. Make it a hard requirement in configure like we do with yasm currently. (I think you can still --disable-... your way out of yasm, it'd be good to double check that.)
Component: General → Toolchains
Assignee

Comment 4

8 months ago
(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #3)
> This is a pain. :-/ It looks like Chromium has had this same conversation.
> 
> In terms of compatibility we'd need to test the various libraries in the
> tree that currently use yasm:
> https://dxr.mozilla.org/mozilla-central/
> search?q=USE_YASM+path%3Abuild&redirect=true
> 
> The biggest ones there are ffvpx, libaom and libjpeg-turbo (the
> config/external/icu/data bit is a trivial bit of assembler).
> 
> If we need to require nasm for the build we'd want to do the following
> things:
> 1. Add toolchain jobs to build nasm binaries in CI. We've got a bunch of
> examples to work from:
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/toolchain/
> linux.yml . This both lets us require nasm for our builds in CI and also
> gives us a way to provide prebuilt binaries for developers.

That shouldn't be necessary for Linux: there are nasm packages in all distros. For CI, we just need to install the nasm package in the build docker images https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/taskcluster/docker/debian7-build/Dockerfile#21. If the version in Debian wheezy is too old, we can backport a package from e.g. Debian stretch quite easily (ping me if you need that).

That leaves Windows for CI, and mac for developers.
Assignee

Comment 6

8 months ago
That sounds easier. We need nasm 2.13 or newer, which is almost certainly not in wheezy.
Reporter

Updated

8 months ago
See Also: → 1493400
Assignee

Updated

8 months ago
Assignee: nobody → tdaede
Assignee

Comment 11

8 months ago
Try of *just* the configure script patch, showing that nasm remains optional with this patchset: https://treeherder.mozilla.org/#/jobs?repo=try&revision=63d58f059c3a3d573bfb9049915a845348c0e80c&selectedJob=208090374
Reporter

Updated

8 months ago
Blocks: 1493400
See Also: 1493400
As I understand it, the current proposal is to have |mach bootstrap| install nasm generally on the user's computer. I would prefer that we install it in the local build sandbox as we do for node.
Assignee

Comment 14

8 months ago
Includes changes to support nasm's stricter include paths.

Supports falling back to yasm if nasm is missing.
Assignee

Comment 15

8 months ago
Try with nasm preferred over yasm for everything (only uses nasm on Linux because win/mac machines don't have it) https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd806a80e8ef5a95e19ba54619081fa5c51642c7
Can we please make sure that this also doesn't break DXR again, provided we manage to unbreak it (bug 1498511) before this lands ?
Reporter

Comment 17

8 months ago
The run in [1] if failing probably because the NASM version on try is 2.10 and the expected version for dav1d is 2.13. On Linux for example I am building locally where I have NASM 2.13.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=208646251&revision=0c4e8f7acdb7054c817b056a1a83e59ee0da845c
Assignee

Comment 18

8 months ago
We had someone else get the same error on that code on nasm 2.11 as well, indicating that we probably need at least 2.12. Oddly, I don't actually hit the error when building dav1d standalone with nasm 2.10 (after configure check has been bypassed) https://github.com/xiph/rav1e/issues/683
Reporter

Comment 21

7 months ago
This is a new run, similar to above, that illustrates better the errors. I would focus on Linux and OSX failures, I haven't verified the asm configuration files for windows and android.
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=210012161&revision=412c0af86864d4e40fb5139237483cd63765d1ed

Comment 23

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ef3f6e06858d
Add nasm to bootstrap scripts. r=glandium

Comment 24

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e4c942b55e5b
Check for nasm in configure script. r=firefox-build-system-reviewers,mshal

Comment 25

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f24864bdfcd
Add USE_NASM mozbuild option. r=firefox-build-system-reviewers,mshal

Comment 27

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6357c007967a
Add nasm 2.12 backport to Debian 7 packages. r=glandium
Backed out for packages for Debian 7 bustage

backout: https://hg.mozilla.org/integration/autoland/rev/d5a0bcaab0aea4bbb5e34428d3a1965507aaaf24

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=6357c007967a3aca099ecd4740873bb4d3451680&selectedJob=212068190

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=212068190&repo=autoland

Processing triggers for man-db ...
debconf: unable to initialize frontend: Dialog
debconf: (No usable dialog-like program is installed, so the dialog based frontend cannot be used. at /usr/share/perl5/Debconf/FrontEnd/Dialog.pm line 76.)
debconf: falling back to frontend: Readline
Setting up python2.6-minimal (2.6.8-1.1+deb7u1) ...
Linking and byte-compiling packages for runtime python2.6...
Setting up python2.6 (2.6.8-1.1+deb7u1) ...
Setting up python3.2-minimal (3.2.3-7+deb7u1) ...
Setting up python3.2 (3.2.3-7+deb7u1) ...
Setting up python3-minimal (3.2.3-6) ...
Setting up python3 (3.2.3-6) ...
running python rtupdate hooks for python3.2...
running python post-rtupdate hooks for python3.2...
+ nproc
+ DEB_BUILD_OPTIONS=parallel=16 nocheck dpkg-buildpackage
dpkg-buildpackage: source package python-zstandard
dpkg-buildpackage: source version 0.9.1-1.deb7moz1
dpkg-buildpackage: source changed by Mozilla build team <dev-builds@lists.mozilla.org>
dpkg-buildpackage: host architecture amd64
 dpkg-source --before-build python-zstandard
dpkg-source: info: using options from python-zstandard/debian/source/options: --extend-diff-ignore=\.egg-info$
dpkg-checkbuilddeps: Unmet build dependencies: dh-python python-all-dev python3-all-dev python-hypothesis python3-hypothesis python-nose python3-nose python-setuptools python3-setuptools
dpkg-buildpackage: warning: build dependencies/conflicts unsatisfied; aborting
dpkg-buildpackage: warning: (Use -d flag to override.)
[taskcluster 2018-11-15 21:53:40.685Z] === Task Finished ===
[taskcluster 2018-11-15 21:53:40.769Z] Artifact "public/build" not found at "/tmp/artifacts"
[taskcluster 2018-11-15 21:53:41.478Z] Unsuccessful task run with exit code: 3 completed in 265.049 seconds
Flags: needinfo?(tdaede)
Assignee

Comment 29

7 months ago
Whoops, did that push in the wrong order.
Flags: needinfo?(tdaede)
Assignee

Comment 30

7 months ago
Actually, no the order shouldn't matter, that looks like an unrelated zstandard build failure?

Comment 31

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1aae4b8f4142
Use nasm when yasm is requested. r=firefox-build-system-reviewers,mshal

Comment 33

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6815082b8e80
Add nasm 2.12 backport to Debian 7 packages. r=glandium

Comment 34

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c62c378d4f58
Add nasm to debian7-build dockerfile. r=glandium
Attachment #9019870 - Attachment is obsolete: true

Comment 36

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b3412b91afe2
Add nasm to debian7-build dockerfile. r=glandium
Backed out  for causing Valgrind build bustage.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=valgrind&fromchange=b3412b91afe2f6fabaf3a9b13c65e8ee45d9c1b4&tochange=e3f7bc3a94a158807643cdc0b3253e469e529f0f&selectedJob=212361938

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=212361938&repo=autoland&lineNumber=46118

Backout link: https://hg.mozilla.org/integration/autoland/rev/e3f7bc3a94a158807643cdc0b3253e469e529f0f

[task 2018-11-17T03:02:24.952Z] 03:02:24     INFO -  14:07.96 TEST-UNEXPECTED-FAIL | valgrind-test | 14 bytes in 1 blocks are definitely lost at malloc / strdup / XREMain::XRE_mainRun / XREMain::XRE_main
[task 2018-11-17T03:02:24.952Z] 03:02:24     INFO -  14:07.96 ==37084== 14 bytes in 1 blocks are definitely lost in loss record 548 of 8,433
[task 2018-11-17T03:02:24.952Z] 03:02:24     INFO -  14:07.96 ==37084==    at 0x4C2B240: malloc+112 (vg_replace_malloc.c:298)
[task 2018-11-17T03:02:24.953Z] 03:02:24     INFO -  14:07.96 ==37084==    by 0x5C7EA81: strdup+33 (strdup.c:43)
[task 2018-11-17T03:02:24.953Z] 03:02:24     INFO -  14:07.96 ==37084==    by 0x12486FD5: XREMain::XRE_mainRun()+2773 (CmdLineAndEnvUtils.h:401)
[task 2018-11-17T03:02:24.953Z] 03:02:24     INFO -  14:07.96 ==37084==    by 0x12487A8F: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)+1151 (nsAppRunner.cpp:4936)
[task 2018-11-17T03:02:24.954Z] 03:02:24     INFO -  14:07.96 ==37084==    by 0x12487F98: XRE_main(int, char**, mozilla::BootstrapConfig const&)+216 (nsAppRunner.cpp:5028)
[task 2018-11-17T03:02:24.954Z] 03:02:24     INFO -  14:07.96 ==37084==    by 0x10EC80: do_main (nsBrowserApp.cpp:233)
[task 2018-11-17T03:02:24.954Z] 03:02:24     INFO -  14:07.96 ==37084==    by 0x10EC80: main+800 (nsBrowserApp.cpp:315)
[task 2018-11-17T03:02:24.954Z] 03:02:24     INFO -  14:07.96 ==37084==
[task 2018-11-17T03:02:24.954Z] 03:02:24     INFO -  14:07.96 {
[task 2018-11-17T03:02:24.955Z] 03:02:24     INFO -  14:07.96    <insert_a_suppression_name_here>
[task 2018-11-17T03:02:24.955Z] 03:02:24     INFO -  14:07.96    Memcheck:Leak
[task 2018-11-17T03:02:24.955Z] 03:02:24     INFO -  14:07.96    match-leak-kinds: definite
[task 2018-11-17T03:02:24.955Z] 03:02:24     INFO -  14:07.96    fun:malloc
[task 2018-11-17T03:02:24.956Z] 03:02:24     INFO -  14:07.96    fun:strdup
[task 2018-11-17T03:02:24.956Z] 03:02:24     INFO -  14:07.96    fun:_ZN7XREMain11XRE_mainRunEv
[task 2018-11-17T03:02:24.956Z] 03:02:24     INFO -  14:07.96    fun:_ZN7XREMain8XRE_mainEiPPcRKN7mozilla15BootstrapConfigE
[task 2018-11-17T03:02:24.956Z] 03:02:24     INFO -  14:07.96    fun:_Z8XRE_mainiPPcRKN7mozilla15BootstrapConfigE
[task 2018-11-17T03:02:24.957Z] 03:02:24     INFO -  14:07.96    fun:do_main
[task 2018-11-17T03:02:24.957Z] 03:02:24     INFO -  14:07.96    fun:main
[task 2018-11-17T03:02:24.957Z] 03:02:24     INFO -  14:07.96 }
Let me note that that last landing (https://hg.mozilla.org/integration/autoland/rev/b3412b91afe2) removed a wayland dependency that was not supposed to be removed!
Assignee

Comment 39

7 months ago
Yeah, that one was a rebasing error on my part (and not lando's).
Flags: needinfo?(tdaede)
Assignee

Updated

7 months ago
Depends on: 1510113
Blocks: 1509724

Comment 41

7 months ago
Pushed by tdaede@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bba74d02a136
Add nasm to debian7-build dockerfile. r=glandium

Comment 42

7 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/997316def2a1
Port bug 1501796 - Add nasm to debian7-build dockerfile. rs=bustage-fix DONTBUILD
You need to log in before you can comment on or make changes to this bug.