If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

libvpx bustage when building on MSVC with /GL

RESOLVED FIXED in mozilla37

Status

()

Core
Build Config
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: RyanVM, Assigned: RyanVM)

Tracking

Trunk
mozilla37
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
libvpx apparently doesn't play nicely with optimizations. When PGO is enabled, asm_enc_offsets.c is specifically included. According to glandium, it would be useful if we were able to disable optimizations for those files in the non-PGO case as well, so filing :)

9:31.98 Finished generating code
9:32.05 Too many sections
9:32.05 c:\mozbuild\src\mozilla-central\objdir-fx-64\media\libvpx\Makefile:433:0: command './host_obj_int_extract.exe gas asm_enc_offsets.obj \
9:32.05 > asm_enc_offsets.asm' failed, return code 1
(Assignee)

Comment 1

4 years ago
s/included/excluded *sigh*
To clarify what i meant, we have a bunch of files that require not being optimized, and that's information we should move in moz.build at some point. It would already be useful to have a config.mk/rules.mk helper for that, and that could be used for the two c files we build in libvpx only to get struct offsets.
(Assignee)

Comment 3

3 years ago
In case anybody's wondering, I checked today and this still happens.
Duplicate of this bug: 939551
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #3)
> In case anybody's wondering, I checked today and this still happens.

You're explicitely adding -GL, right? (because for "normal" PGO, with MOZ_PGO, this is not supposed to happen)

Comment 6

3 years ago
You should be able to build with LTCG without using PGO, though.
And won't this happen with PGO as well? -GL is implied for any PGO build.
So apparently with -GL we get (according to dumpbin) "ANONYMOUS OBJECT" rather than "COFF OBJECT".

In the bytes where COFF would store the number of sections, the file has 0xFFFF instead. So I guess this is unrelated to the many-rdata issue seen in bug 1018402.
Yeah, LTCG-output obj files are not real obj files.
(In reply to Mark Straver from comment #6)
> And won't this happen with PGO as well? -GL is implied for any PGO build.

I think what glandium means is that when you do a PGO build by setting MOZ_PGO, then that also triggers the fixup to not PGO this file.

Comment 10

3 years ago
How does this fixup work? Can it be triggered without pgo build?

Comment 11

3 years ago
I managed to build Firefox with /GL by following the steps in bug 771588:

Modify Makefile.in for libvpx

ifdef VPX_NEED_OBJ_INT_EXTRACT

+# only for MSVC
+ifeq (WINNT_,$(OS_TARGET)_$(GNU_CC))
+vpx_scale_asm_offsets.$(OBJ_SUFFIX): CFLAGS += -GL- 
+endif


vpx_scale_asm_offsets.asm: vpx_scale_asm_offsets.$(OBJ_SUFFIX) $(HOST_PROGRAM)
	./$(HOST_PROGRAM) $(VPX_OIE_FORMAT) $< \
	    $(if $(VPX_AS_CONVERSION),| $(VPX_AS_CONVERSION)) > $@

# Filter out this object, because we don't want to link against it.
# It was generated solely so it could be parsed by obj_int_extract.
OBJS := $(filter-out vpx_scale_asm_offsets.$(OBJ_SUFFIX),$(OBJS))

+# only for MSVC
+ifeq (WINNT_,$(OS_TARGET)_$(GNU_CC))
+vp8_asm_enc_offsets.$(OBJ_SUFFIX): CFLAGS += -GL-
+endif

vp8_asm_enc_offsets.asm: vp8_asm_enc_offsets.$(OBJ_SUFFIX) $(HOST_PROGRAM)
	./$(HOST_PROGRAM) $(VPX_OIE_FORMAT) $< \
	    $(if $(VPX_AS_CONVERSION),| $(VPX_AS_CONVERSION)) > $@

# Filter out this object, because we don't want to link against it.
# It was generated solely so it could be parsed by obj_int_extract.
OBJS := $(filter-out vp8_asm_enc_offsets.$(OBJ_SUFFIX),$(OBJS))

endif
(Assignee)

Comment 12

3 years ago
Still broken with MSVC2013.
As far as I can tell this issue was already fixed in bug 771588 but the fix was lost here: https://hg.mozilla.org/integration/mozilla-inbound/diff/0fc3f0660258/media/libvpx/Makefile.in
(That refactoring was sound for our main build, but it broke people who want to build GL without PGO).

So why don't we just reapply that fix -- that's what comment 11 did. Ryan since you can repro this, want to test it out and post a patch?
(Assignee)

Comment 14

3 years ago
I see where the no_pgo is set in moz.build, but I'm not sure how to make the change shown in the changeset you linked to.
Well my proposal (which glandium might not like) is to leave the no_pgo stuff in moz.build, but do an additional -GL- in Makefile.in for good measure, just like the old days.
(Assignee)

Comment 16

3 years ago
What about something like SOURCES[s].flags += ['-GL-']?
(Assignee)

Comment 17

3 years ago
Yep, that works locally. I'll push it off to try with and without PGO enabled to make sure our build infra doesn't puke either.
(Assignee)

Comment 18

3 years ago
Created attachment 8545647 [details] [diff] [review]
Disable /GL for the libvpx asm files

I've verified that this works locally. I have Try runs in progress both with and without PGO enabled to confirm that it doesn't break anything. I'd like to be able to land this assuming they turn out green :)
Attachment #8545647 - Flags: review?(mh+mozilla)
(Assignee)

Comment 19

3 years ago
Comment on attachment 8545647 [details] [diff] [review]
Disable /GL for the libvpx asm files

Green on Try. Can I get an rs to land please? :)
Attachment #8545647 - Flags: review?(ted)
Attachment #8545647 - Flags: review?(ted) → review+
(Assignee)

Comment 20

3 years ago
Thanks!
https://hg.mozilla.org/integration/mozilla-inbound/rev/b43ac7b3918b
Assignee: nobody → ryanvm
(Assignee)

Updated

3 years ago
Attachment #8545647 - Flags: review?(mh+mozilla)
https://hg.mozilla.org/mozilla-central/rev/b43ac7b3918b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.