From c320a822132223eba5b317314c5cdc001799d423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Mon, 9 Dec 2019 19:22:08 +0100 Subject: [PATCH] SCons: Add 'split_libmodules' option to workaround linker issue The new 'split_libmodules=yes' option is useful to work around linker command line size limitations when linking a huge number of objects. We're currently over 64k chars when linking libmodules.a on Windows with MinGW, which triggers issues as seen in #30892. Even on Linux, we can also reach linker command line size limitations by adding more custom modules. We force this option to True for MinGW on Windows, which fixes #30892. Additional changes to lib splitting: - Fix linking of the split module libs with interdependent symbols, hacking our way into LINKCOM and SHLINKCOM to set the `--start-group` and `--end-group` flags. - Fix Python 3 compatibility in `methods.split_lib()`. - Drop seemingly obsolete condition for 'msys' on 'posix'. - Drop the unnecessary 'split_drivers' as the drivers lib is no longer too big since we moved all thirdparty builds to modules. Co-authored-by: Hein-Pieter van Braam-Stewart --- SConstruct | 3 +-- drivers/SCsub | 10 ++++------ methods.py | 26 +++++++++++++++++++------- modules/SCsub | 2 +- platform/windows/detect.py | 9 ++++++--- 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/SConstruct b/SConstruct index 0e282a1f12c..ce13c8bd2eb 100644 --- a/SConstruct +++ b/SConstruct @@ -68,8 +68,6 @@ env_base.AppendENVPath('PATH', os.getenv('PATH')) env_base.AppendENVPath('PKG_CONFIG_PATH', os.getenv('PKG_CONFIG_PATH')) env_base.disabled_modules = [] env_base.use_ptrcall = False -env_base.split_drivers = False -env_base.split_modules = False env_base.module_version_string = "" env_base.msvc = False @@ -129,6 +127,7 @@ opts.Add(BoolVariable('dev', "If yes, alias for verbose=yes warnings=extra werro opts.Add('extra_suffix', "Custom extra suffix added to the base filename of all generated binary files", '') opts.Add(BoolVariable('vsproj', "Generate a Visual Studio solution", False)) opts.Add(EnumVariable('macports_clang', "Build using Clang from MacPorts", 'no', ('no', '5.0', 'devel'))) +opts.Add(BoolVariable('split_libmodules', "Split intermediate libmodules.a in smaller chunks to prevent exceeding linker command line size (forced to True when using MinGW)", False)) opts.Add(BoolVariable('disable_3d', "Disable 3D nodes for a smaller executable", False)) opts.Add(BoolVariable('disable_advanced_gui', "Disable advanced GUI nodes and behaviors", False)) opts.Add(BoolVariable('no_editor_splash', "Don't use the custom splash screen for the editor", False)) diff --git a/drivers/SCsub b/drivers/SCsub index 583973c0255..d7003a07ced 100644 --- a/drivers/SCsub +++ b/drivers/SCsub @@ -46,9 +46,7 @@ if env['vsproj']: env.AddToVSProject(env.drivers_sources) os.chdir(path) -if env.split_drivers: - env.split_lib("drivers") -else: - env.add_source_files(env.drivers_sources, "*.cpp") - lib = env.add_library("drivers", env.drivers_sources) - env.Prepend(LIBS=[lib]) +env.add_source_files(env.drivers_sources, "*.cpp") + +lib = env.add_library("drivers", env.drivers_sources) +env.Prepend(LIBS=[lib]) diff --git a/methods.py b/methods.py index 86ab7cd9afc..84074db0eb1 100644 --- a/methods.py +++ b/methods.py @@ -307,7 +307,7 @@ def split_lib(self, libname, src_list = None, env_lib = None): else: fname = env.File(f)[0].path fname = fname.replace("\\", "/") - base = string.join(fname.split("/")[:2], "/") + base = "/".join(fname.split("/")[:2]) if base != cur_base and len(list) > max_src: if num > 0: lib = env_lib.add_library(libname + str(num), list) @@ -320,12 +320,6 @@ def split_lib(self, libname, src_list = None, env_lib = None): lib = env_lib.add_library(libname + str(num), list) lib_list.append(lib) - if len(lib_list) > 0: - if os.name == 'posix' and sys.platform == 'msys': - env.Replace(ARFLAGS=['rcsT']) - lib = env_lib.add_library(libname + "_collated", lib_list) - lib_list = [lib] - lib_base = [] env_lib.add_source_files(lib_base, "*.cpp") lib = env_lib.add_library(libname, lib_base) @@ -333,6 +327,24 @@ def split_lib(self, libname, src_list = None, env_lib = None): env.Prepend(LIBS=lib_list) + # When we split modules into arbitrary chunks, we end up with linking issues + # due to symbol dependencies split over several libs, which may not be linked + # in the required order. We use --start-group and --end-group to tell the + # linker that those archives should be searched repeatedly to resolve all + # undefined references. + # As SCons doesn't give us much control over how inserting libs in LIBS + # impacts the linker call, we need to hack our way into the linking commands + # LINKCOM and SHLINKCOM to set those flags. + + if '-Wl,--start-group' in env['LINKCOM'] and '-Wl,--start-group' in env['SHLINKCOM']: + # Already added by a previous call, skip. + return + + env['LINKCOM'] = str(env['LINKCOM']).replace('$_LIBFLAGS', + '-Wl,--start-group $_LIBFLAGS -Wl,--end-group') + env['SHLINKCOM'] = str(env['LINKCOM']).replace('$_LIBFLAGS', + '-Wl,--start-group $_LIBFLAGS -Wl,--end-group') + def save_active_platforms(apnames, ap): diff --git a/modules/SCsub b/modules/SCsub index 42d89d6ce2b..dc0420616c1 100644 --- a/modules/SCsub +++ b/modules/SCsub @@ -16,7 +16,7 @@ for x in env.module_list: env_modules.Append(CPPDEFINES=["MODULE_" + x.upper() + "_ENABLED"]) SConscript(x + "/SCsub") -if env.split_modules: +if env['split_libmodules']: env.split_lib("modules", env_lib = env_modules) else: lib = env_modules.add_library("modules", env.modules_sources) diff --git a/platform/windows/detect.py b/platform/windows/detect.py index b37a21f37fd..500736bd3fa 100644 --- a/platform/windows/detect.py +++ b/platform/windows/detect.py @@ -151,14 +151,14 @@ def setup_msvc_auto(env): env['bits'] = '64' else: env['bits'] = '32' - print(" Found MSVC version %s, arch %s, bits=%s" % (env['MSVC_VERSION'], env['TARGET_ARCH'], env['bits'])) + print("Found MSVC version %s, arch %s, bits=%s" % (env['MSVC_VERSION'], env['TARGET_ARCH'], env['bits'])) if env['TARGET_ARCH'] in ('amd64', 'x86_64'): env["x86_libtheora_opt_vc"] = False def setup_mingw(env): """Set up env for use with mingw""" # Nothing to do here - print("Using Mingw") + print("Using MinGW") pass def configure_msvc(env, manual_msvc_config): @@ -294,7 +294,10 @@ def configure_mingw(env): ## Compiler configuration if (os.name == "nt"): - env['ENV']['TMP'] = os.environ['TMP'] # way to go scons, you can be so stupid sometimes + # Force splitting libmodules.a in multiple chunks to work around + # issues reaching the linker command line size limit, which also + # seem to induce huge slowdown for 'ar' (GH-30892). + env['split_libmodules'] = True else: env["PROGSUFFIX"] = env["PROGSUFFIX"] + ".exe" # for linux cross-compilation