Commit fde2fa0c authored by Nico Weber's avatar Nico Weber Committed by Commit Bot

win: Simplify incremental linking flags setup.

We had a bunch of workarounds to get link.exe not use /incremental
for very large binaries.

Since we no longer use link.exe, remove those workarounds, and
since this removes all reference to a bunch of configs, remove
the configs as well.

Inline all that remains into default_incremental_linking.
(There's more that can be simplified here, but one thing
at a time.)

Bug: 1053958
Change-Id: I4cb123ef1a45fdbcd16d09211e55715095744a46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2079088
Auto-Submit: Nico Weber <thakis@chromium.org>
Reviewed-by: default avatarHans Wennborg <hans@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745169}
parent 70bd5cca
...@@ -514,52 +514,24 @@ config("windowed") { ...@@ -514,52 +514,24 @@ config("windowed") {
# Incremental linking ---------------------------------------------------------- # Incremental linking ----------------------------------------------------------
incremental_linking_on_switch = [ "/INCREMENTAL" ]
if ((!is_debug && !is_component_build) || !use_lld) {
incremental_linking_off_switch = [ "/INCREMENTAL:NO" ]
}
if (use_lld) {
incremental_linking_on_switch += [ "/OPT:NOREF" ]
}
# Enable incremental linking for debug builds and all component builds - any
# builds where performance is not job one.
if (is_debug || is_component_build) {
default_incremental_linking_switch = incremental_linking_on_switch
} else {
default_incremental_linking_switch = incremental_linking_off_switch
}
# Applies incremental linking or not depending on the current configuration. # Applies incremental linking or not depending on the current configuration.
config("default_incremental_linking") { config("default_incremental_linking") {
ldflags = default_incremental_linking_switch # Enable incremental linking for debug builds and all component builds - any
} # builds where performance is not job one.
# TODO(thakis): Always turn this on with lld, no reason not to.
# Explicitly on or off incremental linking if (is_debug || is_component_build) {
config("incremental_linking") { ldflags = [ "/INCREMENTAL" ]
ldflags = incremental_linking_on_switch if (use_lld) {
} # lld doesn't use ilk files and doesn't really have an incremental link
config("no_incremental_linking") { # mode; the only effect of the flag is that the .lib file timestamp isn't
# Projects disable incremental linking to work around ilk file issues with # updated if the .lib doesn't change.
# link.exe. lld doesn't use ilk files and doesn't really have an incremental # TODO(thakis): Why pass /OPT:NOREF for lld, but not otherwise?
# link mode; the only effect of the flag is that the .lib file timestamp isn't # TODO(thakis): /INCREMENTAL is on by default in link.exe, but not in
# updated if the .lib doesn't change. # lld.
if (!use_lld) { ldflags += [ "/OPT:NOREF" ]
ldflags = incremental_linking_off_switch }
}
}
# Some large modules can't handle incremental linking in some situations. This
# config should be applied to large modules to turn off incremental linking
# when it won't work.
config("default_large_module_incremental_linking") {
if (use_lld || symbol_level == 0 ||
(current_cpu == "x86" && is_component_build)) {
# In these configurations, ilk file sizes stay low enough that we can
# link incrementally.
ldflags = default_incremental_linking_switch
} else { } else {
ldflags = incremental_linking_off_switch ldflags = [ "/INCREMENTAL:NO" ]
} }
} }
......
...@@ -398,14 +398,6 @@ if (is_win) { ...@@ -398,14 +398,6 @@ if (is_win) {
configs += [ "//build/config/win:delayloads_not_for_child_dll" ] configs += [ "//build/config/win:delayloads_not_for_child_dll" ]
} }
if (!is_component_build) {
# This is a large module that can't do incremental linking in some
# cases.
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
}
if (use_aura) { if (use_aura) {
deps += [ "//ui/compositor" ] deps += [ "//ui/compositor" ]
} }
......
...@@ -10,11 +10,6 @@ shared_library("verifier_test_dll_1") { ...@@ -10,11 +10,6 @@ shared_library("verifier_test_dll_1") {
"verifier_test_dll.cc", "verifier_test_dll.cc",
"verifier_test_dll_1.def", "verifier_test_dll_1.def",
] ]
# Unconditionally disable incremental linking for these modules so that
# their exports do not go through an ILT jmp stub.
configs -= [ "//build/config/win:default_incremental_linking" ]
configs += [ "//build/config/win:no_incremental_linking" ]
} }
shared_library("verifier_test_dll_2") { shared_library("verifier_test_dll_2") {
...@@ -23,6 +18,4 @@ shared_library("verifier_test_dll_2") { ...@@ -23,6 +18,4 @@ shared_library("verifier_test_dll_2") {
"verifier_test_dll.cc", "verifier_test_dll.cc",
"verifier_test_dll_2.def", "verifier_test_dll_2.def",
] ]
configs -= [ "//build/config/win:default_incremental_linking" ]
configs += [ "//build/config/win:no_incremental_linking" ]
} }
...@@ -1635,9 +1635,6 @@ if (!is_android) { ...@@ -1635,9 +1635,6 @@ if (!is_android) {
# TODO(halyavin) NaCl on Windows can't open debug stub socket in # TODO(halyavin) NaCl on Windows can't open debug stub socket in
# browser process as needed by this test. See http://crbug.com/157312. # browser process as needed by this test. See http://crbug.com/157312.
sources -= [ "../browser/nacl_host/test/gdb_debug_stub_browsertest.cc" ] sources -= [ "../browser/nacl_host/test/gdb_debug_stub_browsertest.cc" ]
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
} }
if (is_linux) { if (is_linux) {
data_deps += [ "//components/nacl/loader:nacl_helper" ] data_deps += [ "//components/nacl/loader:nacl_helper" ]
...@@ -5242,13 +5239,6 @@ test("unit_tests") { ...@@ -5242,13 +5239,6 @@ test("unit_tests") {
deps += [ "//rlz:test_support" ] deps += [ "//rlz:test_support" ]
} }
if (is_win) { if (is_win) {
if (!is_component_build) {
# The PDB gets too large for incremental linking.
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
}
sources += [ sources += [
"../browser/notifications/win/notification_image_retainer_unittest.cc", "../browser/notifications/win/notification_image_retainer_unittest.cc",
"../browser/notifications/win/notification_template_builder_unittest.cc", "../browser/notifications/win/notification_template_builder_unittest.cc",
...@@ -6024,10 +6014,6 @@ if (!is_android) { ...@@ -6024,10 +6014,6 @@ if (!is_android) {
"//ui/resources", "//ui/resources",
] ]
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
libs = [ libs = [
"oleacc.lib", "oleacc.lib",
"runtimeobject.lib", "runtimeobject.lib",
...@@ -6436,10 +6422,6 @@ if (!is_fuchsia && !is_android) { ...@@ -6436,10 +6422,6 @@ if (!is_fuchsia && !is_android) {
"//third_party/wtl", "//third_party/wtl",
"//ui/resources", "//ui/resources",
] ]
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
} }
if (is_chromeos) { if (is_chromeos) {
sources += [ sources += [
...@@ -6505,9 +6487,6 @@ if (!is_fuchsia && !is_android) { ...@@ -6505,9 +6487,6 @@ if (!is_fuchsia && !is_android) {
"//third_party/wtl", "//third_party/wtl",
"//ui/resources", "//ui/resources",
] ]
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
} }
if (toolkit_views) { if (toolkit_views) {
...@@ -6668,10 +6647,6 @@ if (!is_fuchsia && !is_android) { ...@@ -6668,10 +6647,6 @@ if (!is_fuchsia && !is_android) {
"//third_party/wtl", "//third_party/wtl",
"//ui/resources", "//ui/resources",
] ]
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
} }
} }
......
...@@ -625,11 +625,6 @@ if (is_android) { ...@@ -625,11 +625,6 @@ if (is_android) {
if (is_win) { if (is_win) {
deps += [ "//sandbox" ] deps += [ "//sandbox" ]
# This is a large module that can't do incremental linking in some cases.
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
if (win_console_app) { if (win_console_app) {
defines += [ "WIN_CONSOLE_APP" ] defines += [ "WIN_CONSOLE_APP" ]
} else { } else {
......
...@@ -224,18 +224,6 @@ component("core") { ...@@ -224,18 +224,6 @@ component("core") {
"//third_party/blink/renderer/platform/wtf", "//third_party/blink/renderer/platform/wtf",
] ]
if (is_win && is_debug && is_component_build && current_cpu == "x64") {
# Incremental linking fails when the .ilk file gets too large.
# 64-bit debug builds with full symbols trigger this problem, so we turn
# off incremental linking in that configuration.
# For fastest builds, use component release builds without debug
# information.
# VC++ bug filed for 64-bit debug incremental link failures:
# https://connect.microsoft.com/VisualStudio/feedback/details/2846790
configs -= [ "//build/config/win:default_incremental_linking" ]
configs += [ "//build/config/win:no_incremental_linking" ]
}
public_configs = [ ":core_include_dirs" ] public_configs = [ ":core_include_dirs" ]
if (is_mac) { if (is_mac) {
......
...@@ -246,11 +246,6 @@ if (is_android) { ...@@ -246,11 +246,6 @@ if (is_android) {
if (is_win) { if (is_win) {
deps += [ "//sandbox" ] deps += [ "//sandbox" ]
# This is a large module that can't do incremental linking in some cases.
configs -= [ "//build/config/win:default_incremental_linking" ]
configs +=
[ "//build/config/win:default_large_module_incremental_linking" ]
if (win_console_app) { if (win_console_app) {
defines += [ "WIN_CONSOLE_APP" ] defines += [ "WIN_CONSOLE_APP" ]
} else { } else {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment