From fe5d1cc8ff34c163968ca6c9292353b7e341c5e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Tue, 13 Sep 2022 17:01:47 +0200 Subject: [PATCH] SCons: Refactor handling of `production` flag and per-platform LTO defaults Fixup to #63288. See #65583 for the bug report. Co-authored-by: Cyberrebell (cherry picked from commit 35a15e619161798820b2bd6ff46178c5b7ccebcf) --- SConstruct | 61 +++++++++++++++++------------------ platform/android/detect.py | 7 ++-- platform/iphone/detect.py | 9 +++--- platform/javascript/detect.py | 4 +++ platform/osx/detect.py | 12 ++++--- platform/windows/detect.py | 8 +++++ platform/x11/detect.py | 4 +++ 7 files changed, 61 insertions(+), 44 deletions(-) diff --git a/SConstruct b/SConstruct index d16f67065..dad206810 100644 --- a/SConstruct +++ b/SConstruct @@ -125,7 +125,7 @@ opts.Add("arch", "Platform-dependent architecture (arm/arm64/x86/x64/mips/...)", opts.Add(EnumVariable("bits", "Target platform bits", "default", ("default", "32", "64"))) opts.Add(EnumVariable("optimize", "Optimization type", "speed", ("speed", "size", "none"))) opts.Add(BoolVariable("production", "Set defaults to build Pandemonium for use in production", False)) -opts.Add(EnumVariable("lto", "Link-time optimization (for production builds)", "none", ("none", "thin", "full"))) +opts.Add(EnumVariable("lto", "Link-time optimization (production builds)", "none", ("none", "auto", "thin", "full"))) opts.Add(BoolVariable("use_rtti", "Use RTTI", False)) opts.Add(BoolVariable("use_exceptions", "Use exceptions", False)) @@ -405,15 +405,41 @@ if selected_platform in platform_list: env["LINKFLAGS"] = "" env.Append(LINKFLAGS=str(LINKFLAGS).split()) - # Platform specific flags + # Platform specific flags. + # These can sometimes override default options. flag_list = platform_flags[selected_platform] for f in flag_list: if not (f[0] in ARGUMENTS): # allow command line to override platform flags env[f[0]] = f[1] - # Must happen after the flags definition, so that they can be used by platform detect + # 'dev' and 'production' are aliases to set default options if they haven't been + # set manually by the user. + # These need to be checked *after* platform specific flags so that different + # default values can be set (e.g. to keep LTO off for `production` on some platforms). + if env["dev"]: + env["verbose"] = methods.get_cmdline_bool("verbose", True) + env["warnings"] = ARGUMENTS.get("warnings", "extra") + env["werror"] = methods.get_cmdline_bool("werror", True) + if env["production"]: + env["use_static_cpp"] = methods.get_cmdline_bool("use_static_cpp", True) + env["debug_symbols"] = methods.get_cmdline_bool("debug_symbols", False) + # LTO "auto" means we handle the preferred option in each platform detect.py. + env["lto"] = ARGUMENTS.get("lto", "auto") + if not env["tools"] and env["target"] == "debug": + print( + "WARNING: Requested `production` build with `tools=no target=debug`, " + "this will give you a full debug template (use `target=release_debug` " + "for an optimized template with debug features)." + ) + + # Must happen after the flags' definition, as configure is when most flags + # are actually handled to change compile options, etc. detect.configure(env) + # Needs to happen after configure to handle "auto". + if env["lto"] != "none": + print("Using LTO: " + env["lto"]) + # Set our C and C++ standard requirements. # Prepending to make it possible to override # This needs to come after `configure`, otherwise we don't have env.msvc. @@ -428,35 +454,6 @@ if selected_platform in platform_list: # We apply it to CCFLAGS (both C and C++ code) in case it impacts C features. env.Prepend(CCFLAGS=["/std:c++14"]) - # 'dev' and 'production' are aliases to set default options if they haven't been set - # manually by the user. - if env["dev"]: - env["verbose"] = methods.get_cmdline_bool("verbose", True) - env["warnings"] = ARGUMENTS.get("warnings", "extra") - env["werror"] = methods.get_cmdline_bool("werror", True) - if env["production"]: - env["use_static_cpp"] = methods.get_cmdline_bool("use_static_cpp", True) - env["lto"] = ARGUMENTS.get("lto", "full") - env["debug_symbols"] = methods.get_cmdline_bool("debug_symbols", False) - if not env["tools"] and env["target"] == "debug": - print( - "WARNING: Requested `production` build with `tools=no target=debug`, " - "this will give you a full debug template (use `target=release_debug` " - "for an optimized template with debug features)." - ) - if env.msvc: - print( - "WARNING: For `production` Windows builds, you should use MinGW with GCC " - "or Clang instead of Visual Studio, as they can better optimize the " - "GDScript VM in a very significant way. MSVC LTO also doesn't work " - "reliably for our use case." - "If you want to use MSVC nevertheless for production builds, set " - "`debug_symbols=no lto=none` instead of the `production=yes` option." - ) - Exit(255) - if env["lto"] != "none": - print("Using LTO: " + env["lto"]) - # Handle renamed options. if "use_lto" in ARGUMENTS or "use_thinlto" in ARGUMENTS: print("Error: The `use_lto` and `use_thinlto` boolean options have been unified to `lto=`.") diff --git a/platform/android/detect.py b/platform/android/detect.py index 87e3d27b5..36dd4b2c0 100644 --- a/platform/android/detect.py +++ b/platform/android/detect.py @@ -39,9 +39,6 @@ def get_ndk_version(): def get_flags(): return [ ("tools", False), - # Benefits of LTO for Android (size, performance) haven't been clearly established yet. - # So for now we override the default value which may be set when using `production=yes`. - ("lto", "none"), ] # Check if Android NDK version is installed @@ -133,6 +130,10 @@ def configure(env): env.Append(CPPFLAGS=["-UNDEBUG"]) # LTO + + if env["lto"] == "auto": # LTO benefits for Android (size, performance) haven't been clearly established yet. + env["lto"] = "none" + if env["lto"] != "none": if env["lto"] == "thin": env.Append(CCFLAGS=["-flto=thin"]) diff --git a/platform/iphone/detect.py b/platform/iphone/detect.py index 4c962a03d..acebb9814 100644 --- a/platform/iphone/detect.py +++ b/platform/iphone/detect.py @@ -42,9 +42,6 @@ def get_opts(): def get_flags(): return [ ("tools", False), - # Disable by default even if production is set, as it makes linking in Xcode - # on exports very slow and that's not what most users expect. - ("lto", "none"), ] @@ -67,7 +64,11 @@ def configure(env): env.Append(CCFLAGS=["-gdwarf-2", "-O0"]) env.Append(CPPDEFINES=["_DEBUG", ("DEBUG", 1)]) - # LTO + ## LTO + + if env["lto"] == "auto": # Disable by default as it makes linking in Xcode very slow. + env["lto"] = "none" + if env["lto"] != "none": if env["lto"] == "thin": env.Append(CCFLAGS=["-flto=thin"]) diff --git a/platform/javascript/detect.py b/platform/javascript/detect.py index 5189f5af9..2b53d3460 100644 --- a/platform/javascript/detect.py +++ b/platform/javascript/detect.py @@ -94,6 +94,10 @@ def configure(env): env["ENV"] = os.environ # LTO + + if env["lto"] == "auto": # Full LTO for production. + env["lto"] = "full" + if env["lto"] != "none": if env["lto"] == "thin": env.Append(CCFLAGS=["-flto=thin"]) diff --git a/platform/osx/detect.py b/platform/osx/detect.py index d18c25062..89662e23f 100644 --- a/platform/osx/detect.py +++ b/platform/osx/detect.py @@ -35,11 +35,7 @@ def get_opts(): def get_flags(): - return [ - # Benefits of LTO for macOS (size, performance) haven't been clearly established yet. - # So for now we override the default value which may be set when using `production=yes`. - ("lto", "none"), - ] + return [] def configure(env): @@ -132,6 +128,10 @@ def configure(env): env.Append(CPPDEFINES=["__MACPORTS__"]) # hack to fix libvpx MM256_BROADCASTSI128_SI256 define # LTO + + if env["lto"] == "auto": # LTO benefits for macOS (size, performance) haven't been clearly established yet. + env["lto"] = "none" + if env["lto"] != "none": if env["lto"] == "thin": env.Append(CCFLAGS=["-flto=thin"]) @@ -140,6 +140,8 @@ def configure(env): env.Append(CCFLAGS=["-flto"]) env.Append(LINKFLAGS=["-flto"]) + # Sanitizers + if env["use_ubsan"] or env["use_asan"] or env["use_lsan"] or env["use_tsan"]: env.extra_suffix += "s" diff --git a/platform/windows/detect.py b/platform/windows/detect.py index 26eae6cde..0c853baa7 100644 --- a/platform/windows/detect.py +++ b/platform/windows/detect.py @@ -280,6 +280,9 @@ def configure_msvc(env, manual_msvc_config): ## LTO + if env["lto"] == "auto": # No LTO by default for MSVC, doesn't help. + env["lto"] = "none" + if env["lto"] != "none": if env["lto"] == "thin": print("ThinLTO is only compatible with LLVM, use `use_llvm=yes` or `lto=full`.") @@ -394,6 +397,11 @@ def configure_mingw(env): env["x86_libtheora_opt_gcc"] = True + ## LTO + + if env["lto"] == "auto": # Full LTO for production with MinGW. + env["lto"] = "full" + if env["lto"] != "none": if env["lto"] == "thin": if not env["use_llvm"]: diff --git a/platform/x11/detect.py b/platform/x11/detect.py index 599d465ed..ab3da0d43 100644 --- a/platform/x11/detect.py +++ b/platform/x11/detect.py @@ -203,6 +203,10 @@ def configure(env): env.Append(LINKFLAGS=["-fsanitize=memory"]) # LTO + + if env["lto"] == "auto": # Full LTO for production. + env["lto"] = "full" + if env["lto"] != "none": if env["lto"] == "thin": if not env["use_llvm"]: