libvpx bustage when building on MSVC with /GL

RESOLVED FIXED in mozilla37

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: RyanVM, Assigned: RyanVM)

Tracking

Trunk
mozilla37
x86
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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
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.
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)
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.
How does this fixup work? Can it be triggered without pgo build?
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
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?
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.
What about something like SOURCES[s].flags += ['-GL-']?
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.
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)
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+
Attachment #8545647 - Flags: review?(mh+mozilla)
https://hg.mozilla.org/mozilla-central/rev/b43ac7b3918b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.