From 580f0c293f1b75ab4abdca08aa0c8c61989ce5eb Mon Sep 17 00:00:00 2001 From: Mike Shal Date: Mon, 9 Dec 2019 18:03:49 +0000 Subject: [PATCH] Bug 1557788 - Remove OBJS_VAR_SUFFIX & .i_o suffix for instrumented builds; r=firefox-build-system-reviewers,chmanchester In 1-tier PGO builds that shared the objdir between the instrumented and profile-use builds, the instrumented build objects used a different suffix (.i_o) to separate them from the profile-use build (which uses the default .o suffix). These builds are now always in separate objdirs, and don't need special suffix rules anymore. As a bonus, this helps fix an issue with buildid.cpp continually rebuilding because libxul_so.list always lists the inputs as *.o, which don't exist if we're using a .i_o suffix. Make would always re-create buildid.cpp and therefore libxul.so in the instrumented build even when nothing has changed. Differential Revision: https://phabricator.services.mozilla.com/D56115 --HG-- extra : moz-landing-system : lando --- config/config.mk | 17 ------- config/rules.mk | 12 ++--- python/mozbuild/mozbuild/backend/common.py | 8 +--- .../mozbuild/backend/recursivemake.py | 47 ++----------------- python/mozbuild/mozbuild/backend/tup.py | 8 ++-- python/mozbuild/mozbuild/frontend/data.py | 8 ---- python/mozbuild/mozbuild/frontend/emitter.py | 4 -- 7 files changed, 14 insertions(+), 90 deletions(-) diff --git a/config/config.mk b/config/config.mk index e802b558bc81..3660ffb38f73 100644 --- a/config/config.mk +++ b/config/config.mk @@ -435,23 +435,6 @@ endif # ! WINNT # this file OBJ_SUFFIX := $(_OBJ_SUFFIX) -OBJS_VAR_SUFFIX := OBJS - -# PGO builds with GCC and clang-cl build objects with instrumentation in -# a first pass, then objects optimized, without instrumentation, in a -# second pass. If we overwrite the objects from the first pass with -# those from the second, we end up not getting instrumentation data for -# better optimization on incremental builds. As a consequence, we use a -# different object suffix for the first pass. -ifdef MOZ_PROFILE_GENERATE -ifneq (,$(GNU_CC)$(CLANG_CL)) -OBJS_VAR_SUFFIX := PGO_OBJS -ifndef NO_PROFILE_GUIDED_OPTIMIZE -OBJ_SUFFIX := i_o -endif -endif -endif - PLY_INCLUDE = -I$(MOZILLA_DIR)/other-licenses/ply # Enable verbose logs when not using `make -s` diff --git a/config/rules.mk b/config/rules.mk index 94b22f41893f..c3b63e9addbc 100644 --- a/config/rules.mk +++ b/config/rules.mk @@ -462,7 +462,7 @@ $(PROGRAM): $(PROGOBJS) $(STATIC_LIBS) $(EXTRA_DEPS) $(RESFILE) $(GLOBAL_DEPS) $ $(REPORT_BUILD) @$(RM) $@.manifest ifeq (_WINNT,$(GNU_CC)_$(OS_ARCH)) - $(LINKER) -NOLOGO -OUT:$@ -PDB:$(LINK_PDBFILE) -IMPLIB:$(basename $(@F)).lib $(WIN32_EXE_LDFLAGS) $(LDFLAGS) $(MOZ_PROGRAM_LDFLAGS) $($(notdir $@)_$(OBJS_VAR_SUFFIX)) $(RESFILE) $(STATIC_LIBS) $(SHARED_LIBS) $(OS_LIBS) + $(LINKER) -NOLOGO -OUT:$@ -PDB:$(LINK_PDBFILE) -IMPLIB:$(basename $(@F)).lib $(WIN32_EXE_LDFLAGS) $(LDFLAGS) $(MOZ_PROGRAM_LDFLAGS) $($(notdir $@)_OBJS) $(RESFILE) $(STATIC_LIBS) $(SHARED_LIBS) $(OS_LIBS) ifdef MSMANIFEST_TOOL @if test -f $@.manifest; then \ if test -f '$(srcdir)/$(notdir $@).manifest'; then \ @@ -478,7 +478,7 @@ ifdef MSMANIFEST_TOOL fi endif # MSVC with manifest tool else # !WINNT || GNU_CC - $(call EXPAND_CC_OR_CXX,$@) -o $@ $(COMPUTED_CXX_LDFLAGS) $(PGO_CFLAGS) $($(notdir $@)_$(OBJS_VAR_SUFFIX)) $(RESFILE) $(WIN32_EXE_LDFLAGS) $(LDFLAGS) $(STATIC_LIBS) $(MOZ_PROGRAM_LDFLAGS) $(SHARED_LIBS) $(OS_LIBS) + $(call EXPAND_CC_OR_CXX,$@) -o $@ $(COMPUTED_CXX_LDFLAGS) $(PGO_CFLAGS) $($(notdir $@)_OBJS) $(RESFILE) $(WIN32_EXE_LDFLAGS) $(LDFLAGS) $(STATIC_LIBS) $(MOZ_PROGRAM_LDFLAGS) $(SHARED_LIBS) $(OS_LIBS) $(call py_action,check_binary,--target $@) endif # WINNT && !GNU_CC @@ -529,7 +529,7 @@ endif $(SIMPLE_PROGRAMS): %$(BIN_SUFFIX): %.$(OBJ_SUFFIX) $(STATIC_LIBS) $(EXTRA_DEPS) $(GLOBAL_DEPS) $(REPORT_BUILD) ifeq (_WINNT,$(GNU_CC)_$(OS_ARCH)) - $(LINKER) -nologo -out:$@ -pdb:$(LINK_PDBFILE) $($@_$(OBJS_VAR_SUFFIX)) $(WIN32_EXE_LDFLAGS) $(LDFLAGS) $(MOZ_PROGRAM_LDFLAGS) $(STATIC_LIBS) $(SHARED_LIBS) $(OS_LIBS) + $(LINKER) -nologo -out:$@ -pdb:$(LINK_PDBFILE) $($@_OBJS) $(WIN32_EXE_LDFLAGS) $(LDFLAGS) $(MOZ_PROGRAM_LDFLAGS) $(STATIC_LIBS) $(SHARED_LIBS) $(OS_LIBS) ifdef MSMANIFEST_TOOL @if test -f $@.manifest; then \ $(MT) -NOLOGO -MANIFEST $@.manifest -OUTPUTRESOURCE:$@\;1; \ @@ -539,7 +539,7 @@ ifdef MSMANIFEST_TOOL fi endif # MSVC with manifest tool else - $(call EXPAND_CC_OR_CXX,$@) $(COMPUTED_CXX_LDFLAGS) $(PGO_CFLAGS) -o $@ $($@_$(OBJS_VAR_SUFFIX)) $(WIN32_EXE_LDFLAGS) $(LDFLAGS) $(STATIC_LIBS) $(MOZ_PROGRAM_LDFLAGS) $(SHARED_LIBS) $(OS_LIBS) + $(call EXPAND_CC_OR_CXX,$@) $(COMPUTED_CXX_LDFLAGS) $(PGO_CFLAGS) -o $@ $($@_OBJS) $(WIN32_EXE_LDFLAGS) $(LDFLAGS) $(STATIC_LIBS) $(MOZ_PROGRAM_LDFLAGS) $(SHARED_LIBS) $(OS_LIBS) $(call py_action,check_binary,--target $@) endif # WINNT && !GNU_CC @@ -568,7 +568,7 @@ endif $(LIBRARY): $(OBJS) $(STATIC_LIBS) $(EXTRA_DEPS) $(GLOBAL_DEPS) $(REPORT_BUILD) $(RM) $(REAL_LIBRARY) - $(AR) $(AR_FLAGS) $($@_$(OBJS_VAR_SUFFIX)) + $(AR) $(AR_FLAGS) $($@_OBJS) $(WASM_ARCHIVE): $(CWASMOBJS) $(CPPWASMOBJS) $(STATIC_LIBS) $(EXTRA_DEPS) $(GLOBAL_DEPS) $(REPORT_BUILD_VERBOSE) @@ -609,7 +609,7 @@ $(SHARED_LIBRARY): $(OBJS) $(RESFILE) $(STATIC_LIBS) $(EXTRA_DEPS) $(GLOBAL_DEPS ifndef INCREMENTAL_LINKER $(RM) $@ endif - $(MKSHLIB) $($@_$(OBJS_VAR_SUFFIX)) $(RESFILE) $(LDFLAGS) $(STATIC_LIBS) $(SHARED_LIBS) $(EXTRA_DSO_LDOPTS) $(MOZ_GLUE_LDFLAGS) $(OS_LIBS) + $(MKSHLIB) $($@_OBJS) $(RESFILE) $(LDFLAGS) $(STATIC_LIBS) $(SHARED_LIBS) $(EXTRA_DSO_LDOPTS) $(MOZ_GLUE_LDFLAGS) $(OS_LIBS) $(call py_action,check_binary,--target $@) ifeq (_WINNT,$(GNU_CC)_$(OS_ARCH)) diff --git a/python/mozbuild/mozbuild/backend/common.py b/python/mozbuild/mozbuild/backend/common.py index 9496cbdf7301..dd8df396868a 100644 --- a/python/mozbuild/mozbuild/backend/common.py +++ b/python/mozbuild/mozbuild/backend/common.py @@ -230,7 +230,6 @@ class CommonBackend(BuildBackend): shared_libs = [] static_libs = [] objs = [] - no_pgo_objs = [] seen_objs = set() seen_libs = set() @@ -242,11 +241,6 @@ class CommonBackend(BuildBackend): seen_objs.add(o) objs.append(o) - # This is slightly odd, but for consistency with the - # recursivemake backend we don't replace OBJ_SUFFIX if any - # object in a library has `no_pgo` set. - if lib.no_pgo_objs or lib.no_pgo: - no_pgo_objs.append(o) def expand(lib, recurse_objs, system_libs): if isinstance(lib, (HostLibrary, StaticLibrary, @@ -289,7 +283,7 @@ class CommonBackend(BuildBackend): seen_libs.add(lib) os_libs.append(lib) - return (objs, no_pgo_objs, shared_libs, os_libs, static_libs) + return (objs, shared_libs, os_libs, static_libs) def _make_list_file(self, kind, objdir, objs, name): if not objs: diff --git a/python/mozbuild/mozbuild/backend/recursivemake.py b/python/mozbuild/mozbuild/backend/recursivemake.py index 59372a4eedf2..ccbfe9e6f901 100644 --- a/python/mozbuild/mozbuild/backend/recursivemake.py +++ b/python/mozbuild/mozbuild/backend/recursivemake.py @@ -1345,43 +1345,14 @@ class RecursiveMakeBackend(MakeBackend): return os.path.normpath(mozpath.join(mozpath.relpath(lib.objdir, obj.objdir), name)) - objs, no_pgo_objs, shared_libs, os_libs, static_libs = self._expand_libs(obj) + objs, shared_libs, os_libs, static_libs = self._expand_libs(obj) obj_target = obj.name if isinstance(obj, Program): obj_target = self._pretty_path(obj.output_path, backend_file) - profile_gen_objs = [] - - if obj.KIND == 'target': - is_unit_test = isinstance(obj, BaseProgram) and obj.is_unit_test - - doing_pgo = self.environment.substs.get('MOZ_PGO') - obj_suffix_change_needed = (self.environment.substs.get('GNU_CC') or - self.environment.substs.get('CLANG_CL')) - if doing_pgo and obj_suffix_change_needed: - # We use a different OBJ_SUFFIX for the profile generate phase on - # systems where the pgo generate phase requires instrumentation - # that can only be removed by recompiling objects. These get - # picked up via OBJS_VAR_SUFFIX in config.mk. - if not is_unit_test and not isinstance(obj, SimpleProgram): - profile_gen_objs = [o if o in no_pgo_objs else '%s.%s' % - (mozpath.splitext(o)[0], 'i_o') for o in objs] - - def write_obj_deps(target, objs_ref, pgo_objs_ref): - if pgo_objs_ref: - backend_file.write('ifdef MOZ_PROFILE_GENERATE\n') - backend_file.write('%s: %s\n' % (target, pgo_objs_ref)) - backend_file.write('else\n') - backend_file.write('%s: %s\n' % (target, objs_ref)) - backend_file.write('endif\n') - else: - backend_file.write('%s: %s\n' % (target, objs_ref)) - objs_ref = ' \\\n '.join(os.path.relpath(o, obj.objdir) for o in objs) - pgo_objs_ref = ' \\\n '.join(os.path.relpath(o, obj.objdir) - for o in profile_gen_objs) # Don't bother with a list file if we're only linking objects built # in this directory or building a real static library. This # accommodates clang-plugin, where we would otherwise pass an @@ -1393,10 +1364,7 @@ class RecursiveMakeBackend(MakeBackend): obj.no_expand_lib): backend_file.write_once('%s_OBJS := %s\n' % (obj.name, objs_ref)) - if profile_gen_objs: - backend_file.write_once('%s_PGO_OBJS := %s\n' % (obj.name, - pgo_objs_ref)) - write_obj_deps(obj_target, objs_ref, pgo_objs_ref) + backend_file.write('%s: %s\n' % (obj_target, objs_ref)) elif not isinstance(obj, (HostLibrary, StaticLibrary, SandboxedWasmLibrary)): list_file_path = '%s.list' % obj.name.replace('.', '_') @@ -1405,16 +1373,7 @@ class RecursiveMakeBackend(MakeBackend): backend_file.write_once('%s_OBJS := %s\n' % (obj.name, list_file_ref)) backend_file.write_once('%s: %s\n' % (obj_target, list_file_path)) - if profile_gen_objs: - pgo_list_file_path = '%s_pgo.list' % obj.name.replace('.', '_') - pgo_list_file_ref = self._make_list_file(obj.KIND, obj.objdir, - profile_gen_objs, - pgo_list_file_path) - backend_file.write_once('%s_PGO_OBJS := %s\n' % - (obj.name, pgo_list_file_ref)) - backend_file.write_once('%s: %s\n' % (obj_target, - pgo_list_file_path)) - write_obj_deps(obj_target, objs_ref, pgo_objs_ref) + backend_file.write('%s: %s\n' % (obj_target, objs_ref)) for lib in shared_libs: assert obj.KIND != 'host' diff --git a/python/mozbuild/mozbuild/backend/tup.py b/python/mozbuild/mozbuild/backend/tup.py index fb9303e663c9..16fa5d8bf154 100644 --- a/python/mozbuild/mozbuild/backend/tup.py +++ b/python/mozbuild/mozbuild/backend/tup.py @@ -416,7 +416,7 @@ class TupBackend(CommonBackend): ['-o', shlib.lib_name] ) - objs, _, shared_libs, os_libs, static_libs = self._expand_libs(shlib) + objs, shared_libs, os_libs, static_libs = self._expand_libs(shlib) static_libs = self._lib_paths(backend_file.objdir, static_libs) shared_libs = self._lib_paths(backend_file.objdir, shared_libs) @@ -470,7 +470,7 @@ class TupBackend(CommonBackend): def _gen_program(self, backend_file, prog): cc_or_cxx = 'CXX' if prog.cxx_link else 'CC' - objs, _, shared_libs, os_libs, static_libs = self._expand_libs(prog) + objs, shared_libs, os_libs, static_libs = self._expand_libs(prog) static_libs = self._lib_paths(backend_file.objdir, static_libs) shared_libs = self._lib_paths(backend_file.objdir, shared_libs) @@ -522,7 +522,7 @@ class TupBackend(CommonBackend): self._gen_host_program(backend_file, p) def _gen_host_program(self, backend_file, prog): - _, _, _, extra_libs, _ = self._expand_libs(prog) + _, _, extra_libs, _ = self._expand_libs(prog) objs = prog.objs if isinstance(prog, HostSimpleProgram): @@ -567,7 +567,7 @@ class TupBackend(CommonBackend): backend_file.environment.substs['AR_FLAGS'].replace('$@', '%o') ] - objs, _, shared_libs, _, static_libs = self._expand_libs(backend_file.static_lib) + objs, shared_libs, _, static_libs = self._expand_libs(backend_file.static_lib) static_libs = self._lib_paths(backend_file.objdir, static_libs) shared_libs = self._lib_paths(backend_file.objdir, shared_libs) diff --git a/python/mozbuild/mozbuild/frontend/data.py b/python/mozbuild/mozbuild/frontend/data.py index 2c99f60ea5ca..010f59833527 100644 --- a/python/mozbuild/mozbuild/frontend/data.py +++ b/python/mozbuild/mozbuild/frontend/data.py @@ -406,8 +406,6 @@ class Linkable(ContextDerived): 'lib_defines', 'linked_libraries', 'linked_system_libs', - 'no_pgo_sources', - 'no_pgo', 'sources', ) @@ -418,8 +416,6 @@ class Linkable(ContextDerived): self.linked_system_libs = [] self.lib_defines = Defines(context, {}) self.sources = defaultdict(list) - self.no_pgo_sources = [] - self.no_pgo = False def link_library(self, obj): assert isinstance(obj, BaseLibrary) @@ -468,10 +464,6 @@ class Linkable(ContextDerived): """Can be overridden by a base class for custom behavior.""" return self.config.substs.get('OBJ_SUFFIX', '') - @property - def no_pgo_objs(self): - return self._get_objs(self.no_pgo_sources) - @property def objs(self): return self._get_objs(self.source_files()) diff --git a/python/mozbuild/mozbuild/frontend/emitter.py b/python/mozbuild/mozbuild/frontend/emitter.py index 2e43e5e4b275..7c6bed589a2a 100644 --- a/python/mozbuild/mozbuild/frontend/emitter.py +++ b/python/mozbuild/mozbuild/frontend/emitter.py @@ -1032,10 +1032,6 @@ class TreeMetadataEmitter(LoggingMixin): for target_var in ('SOURCES', 'UNIFIED_SOURCES'): for suffix, srcs in ctxt_sources[target_var].items(): linkable.sources[suffix] += srcs - if no_pgo_sources: - linkable.no_pgo_sources = no_pgo_sources - elif no_pgo: - linkable.no_pgo = True for host_linkable in host_linkables: for suffix, srcs in ctxt_sources['HOST_SOURCES'].items(): host_linkable.sources[suffix] += srcs