Closed Bug 1501796 Opened 6 years ago Closed 6 years ago

Add NASM to our build system

Categories

(Firefox Build System :: Toolchains, enhancement)

Product:
Firefox Build System
Component:
Toolchains
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox65 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla65
Tracking Flags:
Tracking Status
firefox65 --- fixed

People

(Reporter: achronop, Assigned: TD-Linux)

References

Details

Attachments

(6 files, 1 obsolete file)

Bug 1501796 - Add nasm to debian7-build dockerfile.
46 bytes, text/x-phabricator-request
Details | Review
Bug 1501796 - Add nasm to bootstrap scripts.
46 bytes, text/x-phabricator-request
Details | Review
Bug 1501796 - Check for nasm in configure script.
46 bytes, text/x-phabricator-request
Details | Review
Bug 1501796 - Add USE_NASM mozbuild option.
46 bytes, text/x-phabricator-request
Details | Review
Bug 1501796 - Use nasm when yasm is requested.
46 bytes, text/x-phabricator-request
Details | Review
Bug 1501796: Add nasm 2.12 backport to Debian 7 packages.
47 bytes, text/x-phabricator-request
Details | Review
dav1d upstream uses NASM and will simplify the import of the library.
yasm upstream is dead and we should switch to nasm for avx-512 support
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
See Also: → https://bugs.chromium.org/p/chromium/issues/detail?id=766721
(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.
That sounds easier. We need nasm 2.13 or newer, which is almost certainly not in wheezy.
See Also: → 1493400
Assignee: nobody → tdaede
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
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.
Includes changes to support nasm's stricter include paths. Supports falling back to yasm if nasm is missing.
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 ?
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
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
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
Pushed by tdaede@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ef3f6e06858d Add nasm to bootstrap scripts. r=glandium
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
Pushed by tdaede@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2f24864bdfcd Add USE_NASM mozbuild option. r=firefox-build-system-reviewers,mshal
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)
Whoops, did that push in the wrong order.
Flags: needinfo?(tdaede)
Actually, no the order shouldn't matter, that looks like an unrelated zstandard build failure?
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
Pushed by tdaede@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6815082b8e80 Add nasm 2.12 backport to Debian 7 packages. r=glandium
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
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!
Yeah, that one was a rebasing error on my part (and not lando's).
Flags: needinfo?(tdaede)
Depends on: 1510113
Blocks: 1509724
Pushed by tdaede@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bba74d02a136 Add nasm to debian7-build dockerfile. r=glandium
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.

Attachment

General

Creator:
Created:
Updated:
Size: