Closed
Bug 934984
Opened 11 years ago
Closed 10 years ago
libvpx bustage when building on MSVC with /GL
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla37
People
(Reporter: RyanVM, Assigned: RyanVM)
References
Details
Attachments
(1 file)
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
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•11 years ago
|
||
s/included/excluded *sigh*
Comment 2•11 years ago
|
||
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•11 years ago
|
||
In case anybody's wondering, I checked today and this still happens.
Comment 5•10 years ago
|
||
(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•10 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.
Comment 8•10 years ago
|
||
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•10 years ago
|
||
How does this fixup work? Can it be triggered without pgo build?
Comment 11•10 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•10 years ago
|
||
Still broken with MSVC2013.
Comment 13•10 years ago
|
||
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•10 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.
Comment 15•10 years ago
|
||
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•10 years ago
|
||
What about something like SOURCES[s].flags += ['-GL-']?
Assignee | ||
Comment 17•10 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•10 years ago
|
||
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•10 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)
Updated•10 years ago
|
Attachment #8545647 -
Flags: review?(ted) → review+
Assignee | ||
Comment 20•10 years ago
|
||
Assignee: nobody → ryanvm
Assignee | ||
Updated•10 years ago
|
Attachment #8545647 -
Flags: review?(mh+mozilla)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•