Closed Bug 1413257 Opened 8 years ago Closed 8 years ago

Linux 64 builds broken: mozilla/media/ffvpx/libavutil/x86/float_dsp.asm:405: error: instruction expected after label

Categories

(Thunderbird :: Build Config, defect)

Product:
Thunderbird
Component:
Build Config
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 58.0

People

(Reporter: jorgk-bmo, Assigned: tomprince)

References

Details

(Whiteboard: [Thunderbird-testfailure: B Linux64])

Attachments

(1 file)

Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot;
59 bytes, text/x-review-board-request
rillian
: review+
jya
: review+
Details
/builds/slave/tb-c-cen-l64-00000000000000000/build/mozilla/media/ffvpx/libavutil/x86/float_dsp.asm:405: error: instruction expected after label /builds/slave/tb-c-cen-l64-00000000000000000/build/mozilla/media/ffvpx/libavutil/x86/float_dsp.asm:405: error: instruction expected after label /builds/slave/tb-c-cen-l64-00000000000000000/build/mozilla/media/ffvpx/libavutil/x86/float_dsp.asm:405: error: undefined symbol `vpermps.loop' (first use) /builds/slave/tb-c-cen-l64-00000000000000000/build/mozilla/media/ffvpx/libavutil/x86/float_dsp.asm:405: error: (Each undefined symbol is reported only once.) Apparently cause by bug 1412339. Jean-Yves and JW: Any idea why that doesn't compile for Thunderbird?
Flags: needinfo?(jyavenard)
Flags: needinfo?(jwwang)
It doesn't compile because we have a too old version of yasm (or something) on the buildbot machines that Thunderbird is using to support AVX2 which was turned on in Bug 1412339. The question is whether it possible to conditionally disable that support.
Time to move to TaskCluster, no? Also for bug 1380171 (stylo disabled in bug 1350011).
Flags: needinfo?(jyavenard)
Flags: needinfo?(jwwang)
Will this disable AVX2 also on local builds where yasm may be of proper updated version?
This only disables it if a configure option is passed, which I'll add to the appropriate thunderbird configs.
Assignee: nobody → mozilla
Depends on: 1412339
(In reply to Tom Prince [:tomprince] from comment #1) > It doesn't compile because we have a too old version of yasm (or something) > on the buildbot machines that Thunderbird is using to support AVX2 which was > turned on in Bug 1412339. The question is whether it possible to > conditionally disable that support. Unfortunately there's no support for dynamically changing the configuration, that would need to be added.
That's what the patch I submitted for review does.
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200156 ::: media/ffvpx/config_unix64.asm:45 (Diff revision 2) > %define HAVE_AMD3DNOW 1 > %define HAVE_AMD3DNOWEXT 1 > %define HAVE_AVX 1 > +%ifdef MOZ_HAVE_AVX2 > %define HAVE_AVX2 1 > +%endif You must have an else to set the value to zero.
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200176 ::: media/ffvpx/ffvpxcommon.mozbuild:16 (Diff revision 3) > if CONFIG['FFVPX_ASFLAGS']: > USE_YASM = True > + > + if CONFIG['MOZ_FFVPX_AVX2']: > + DEFINES['MOZ_HAVE_AVX2'] = True > + ASFLAGS += ['-DMOZ_HAVE_AVX2'] old-configure.in already defines `HAVE_X86_AVX2`. Can you use that instead? Would avoid needing a new configure switch. The bug suggests the problem is yasm, not the compiler, which suggests our yasm version check is out of date, too.
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200176 > old-configure.in already defines `HAVE_X86_AVX2`. Can you use that instead? Would avoid needing a new configure switch. > > The bug suggests the problem is yasm, not the compiler, which suggests our yasm version check is out of date, too. Unfortunatelly, it looks like the compiler does support AVX, it is just the version of yasm in the build environment. So > The bug suggests the problem is yasm, not the compiler, which suggests our yasm version check is out of date, too. That may be, but I'd appreciate if it didn't become a hard error until later. I'm working getting TB to using taskcluster, and I'd rather focus on that rather than figure out how to get newer yasm onto buildbot. That does make me think that it might be possible to detect old versions of yasm and disable AVX2 for ffvpx based on that?
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200176 > Unfortunatelly, it looks like the compiler does support AVX, it is just the version of yasm in the build environment. So > > > The bug suggests the problem is yasm, not the compiler, which suggests our yasm version check is out of date, too. > > That may be, but I'd appreciate if it didn't become a hard error until later. I'm working getting TB to using taskcluster, and I'd rather focus on that rather than figure out how to get newer yasm onto buildbot. > > That does make me think that it might be possible to detect old versions of yasm and disable AVX2 for ffvpx based on that? build/moz.configure/toolchain.configure defines _YASM_MAJOR_VERSION and _YASM_MINOR_VERSION for old-configure.in. You could add a `set_config` call to those to have them available in `moz.build` files. You could also upload a newer yasm to tooltool and define `YASM` in your mozconfig, which has the advantage of there being less to dismantle when you get builds ported to taskcluster.
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200176 > build/moz.configure/toolchain.configure defines _YASM_MAJOR_VERSION and _YASM_MINOR_VERSION for old-configure.in. You could add a `set_config` call to those to have them available in `moz.build` files. > > You could also upload a newer yasm to tooltool and define `YASM` in your mozconfig, which has the advantage of there being less to dismantle when you get builds ported to taskcluster. > You could also upload a newer yasm to tooltool and define YASM in your mozconfig, which has the advantage of there being less to dismantle when you get builds ported to taskcluster. I'd rather not have to figure out how to package a newer YASM for centos. And I'll have plenty of time once I've migrated, but have a deadline of 59 for getting off buildbot.
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200360 it would be better to reset (#undef) and reset the value in config.h instead, leaving the config_unix64.h alone. otherwise its making it much more difficult to maintain changes. those .h files are auto-generated, and those chamges will be automatically overwritten whenever we update the file. i don't think the value in the .asm is used. an alternative would be for thunderbird to not set the FFVPX_ASFLAGS and usae config_unix32.h which default to a C only version. or you could go one step further, which ultimately is likely the best: you could even not set MOZ_FFVPX altogether (in configure.in), this will use libvpx instead or the system ffmpeg if found, thst way no files touched, easier to support with little performance change for thunderbir
Attachment #8923930 - Flags: review?(jyavenard) → review-
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200360 > you could go one step further, which ultimately is likely the best: you could even not set MOZ_FFVPX altogether (in configure.in), this will use libvpx instead or the system ffmpeg if found, I don't fully understand the implications of this option. We definitely can't use a system ffmpeg as we are compiling on linux environment that is difficult to upgrade[1]. I'm not familiar with Gecko's media support so I am unsure what the impact on supported formats and performance would be, just using the in-tree code if I disable MOZ_FFVPX. [1] I'm busy working on updating our build environment and hope to be done by the time 59 branches. > I don't think the value in the .asm is used. [This](https://treeherder.mozilla.org/logviewer.html#?job_id=141265999&repo=comm-central&lineNumber=6025) suggests it does (which is an error coming from [here](https://dxr.mozilla.org/mozilla-central/source/media/ffvpx/libavutil/x86/float_dsp.asm#405). > otherwise its making it much more difficult to maintain changes. those .h files are auto-generated, and those chamges will be automatically overwritten whenever we update the file. I definitely view this fix (or any other) as a temporary measure which won't be necesary for long[2], and I'd be happy if you did just overwrite the files when you update, and then I can folow up by relanding the equivalent of this patch when TB breaks. [2] If TB has not migrated by the time 59 branches, we'll have much bigger issues to deal with.
I'll have another approach possible once bug 1295886 goes in. I've added an option there to only have a flac decoder, disabling both the vp8 and vp9 decoder. With this flag on, there will no longer be a compilation issue there.... Then it will be just a matter of setting that config option to true when building for thunderbird. would that be an acceptable solution?
That depends on the ETA of bug 1295886. I don't see a patch with r? or r+ there yet.
Attachment #8923930 - Flags: review?(jyavenard)
Attachment #8923930 - Flags: review?(core-build-config-reviews)
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200574 Looks reasonble to me.
Attachment #8923930 - Flags: review?(giles) → review+
Comment on attachment 8923930 [details] Bug 1413257: Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; https://reviewboard.mozilla.org/r/195094/#review200700
Attachment #8923930 - Flags: review?(jyavenard) → review+
Attachment #8923930 - Flags: review?(core-build-config-reviews)
(In reply to Jorg K (GMT+2) from comment #20) > That depends on the ETA of bug 1295886. I don't see a patch with r? or r+ > there yet. I should be able to have it available in a couple of hours... though apparently no longer necessary...
Attachment #8923930 - Flags: review?(core-build-config-reviews)
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/integration/autoland/rev/769dd8ce70e1 Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; r=jya,rillian
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/integration/autoland/rev/d1d318017187 Allow disabling ffvpx's AVX2 support on linux64 for Thunderbird's buildbot; r=jya,rillian
Fixed flake8 failures and submitted to autoland again.
https://hg.mozilla.org/mozilla-central/rev/d1d318017187
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 58.0
unfortunate the Build review request got dismissed...
"Build" is a shared review queue (see https://groups.google.com/forum/#!topic/mozilla.dev.platform/yM1_GCwP8Mo). The build review was done by :rillian who is a build peer (https://wiki.mozilla.org/Modules/Core#Build_Config).
Flags: needinfo?(mozilla)
Depends on: 1414298
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: