Confirmed.

Synced to ce2c18d86d5188e2d5fc4b2fa5d17c465382813d (June 14th). Used supplied args.gn and just built mksnapshot.exe:

ninja -v -d keeprsp -C out\gn_bug_621236 mksnapshot.exe
ninja: Entering directory `out\gn_bug_621236'
[0 processes, 830/830 @ 8.3/s : 99.713s ] LINK mksnapshot.exe
LINK : warning LNK4075: ignoring '/INCREMENTAL' due to '/OPT:ICF' specification

/OPT:ICF implies linker optimizations, /INCREMENTAL implies no linker optimizations. There has been some talk of turning on /INCREMENTAL for component builds even in release but I'm not sure if that has happened.

It's not immediately clear why this happens for mksnapshot.exe and not for everything. On the same build base.dll.rsp contains /INCREMENTAL but not /OPT:ICF, so /OPT:ICF appears to be the outlier.

btw it happened with v8 later in the build as well.

[11802/17742] LINK(DLL) v8.dll v8.dll.lib
LINK : warning LNK4075: ignoring '/INCREMENTAL' due to '/OPT:ICF' specification
[17742/17742] STAMP obj/chrome/chrome.stamp

Something v8 specific I guess - mksnapshot and and v8 are defined in the same BUILD.gn file.

Also happens to me for various other things, e.g. I just got it for mkpeephole.exe.  I see a couple of these warnings every build.

Would be nice to fix this.  We have compiler warnings-as-errors on, is there a linker warnings-as-errors setting?  If so shooting to enable that seems good.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7bf1119ce51d5e5b489c780bbacc332eabdd6f07

commit 7bf1119ce51d5e5b489c780bbacc332eabdd6f07
Author: brucedawson <brucedawson@chromium.org>
Date: Wed Sep 14 20:36:45 2016

Enable incremental linking in release component builds

Component builds are, by definition, not optimized for maximum
performance. So incremental linking - which trades a little bit of
performance for much faster build times - is appropriate in release
component builds, as well as in debug builds.

This change enables incremental linking in release component builds.
This also requires turning off /OPT:ICF and /PROFILE in component
builds because they are incompatible with /incremental, but they are
performance related flags so they are also not important for component
builds.

blink_core.dll cannot be incrementally linked in debug x64 component
builds, but it *can* be incrementally linked in release x64 component
builds (and debug/release x86 component builds), and this change
enables incremental linking of blink_core.dll in all supported
configurations.

This drops the chrome rebuild time after touching
third_party\WebKit\Source\core\html\AutoplayExperimentHelper.cpp from
~260 seconds to 9 seconds. The chrome rebuild time after touching
base\win\registry.cc drops from ~200 seconds to less than 2 seconds,
with 21 of 24 link steps skipped entirely.

This also fixes some /INCREMENTAL /OPT:ICF mismatch warnings.

BUG= 608801 , 621236 

Review-Url: https://codereview.chromium.org/2331373006
Cr-Commit-Position: refs/heads/master@{#418660}

[modify] https://crrev.com/7bf1119ce51d5e5b489c780bbacc332eabdd6f07/build/config/compiler/BUILD.gn
[modify] https://crrev.com/7bf1119ce51d5e5b489c780bbacc332eabdd6f07/build/config/win/BUILD.gn
[modify] https://crrev.com/7bf1119ce51d5e5b489c780bbacc332eabdd6f07/third_party/WebKit/Source/core/BUILD.gn

Sigh... apparently not fixed.

is_component_build = false
is_debug = true
is_clang = true
symbol_level = 1

Also happens without is_clang = true and without symbol_level = 1.
It requires is_component_build = false and is_debug = true

I'm not sure if this regressed again or if the original fix was incorrect.

Re #7: IIUC the issue with mksnapshot/mkpeephole after your patch is that in non-component Debug builds we're setting both the incremental link and the OPT:ICF flags, which are incompatible.  We enable incremental link in debug OR component builds, but we enable OPT:ICF in any non-component build; so do we just need to correct the condition for OPT:ICF to be (!is_debug && !is_component_build)?

More generally, could we prioritize the configuration options so that whether or not OPT:ICF is set can depend more directly on whether incremental linking is enabled (or vice versa)?

It is a error to try to use /OPT:ICF in a debug build - correcting that would be a good idea.

> do we just need to correct the condition for OPT:ICF to be (!is_debug && !is_component_build)?

Yes.

Are these executables explicitly setting /OPT:ICF themselves? The logic should mostly be centralized in order to avoid this issue.

There are two sets of logic; some from build/compiler/BUILD.gn, which is the bit that is setting /OPT:ICF in the default optimized ldflags, and some from build/win/BUILD.gn, which is the bit that is setting the incremental link flag.

I'd have expected that we could completely remove the stuff from build/compiler/BUILD.gn that sets those, since in principle it just duplicates what you added in win/BUILD.gn, but when I removed it mkpeephole's link line lost the /OPT:ICF in Release non-component builds, so there's still something else going on.

OK, so what's going on here is:

- All code under src/v8/ picks up the optimize_max config, and hence the /OPT:ICF, even in is_debug builds, because V8 has optimize-in-debug set by default.
- Chromium code which uses optimize_max does so dependent on is_debug, so won't pick up /OPT:ICF.
- Because the build is Debug, it picks up /INCREMENTAL for most targets[1], even though it's non-component.

[1] Some targets, such as chrome.dll, explicitly opt-in to a special incremental linker default for "large" targets, which in this particular configuration defaults to off.

I have a CL out to patch up the compiler:optimize_max logic to match win:compiler, but I wonder if we can restructure things to be harder to get out of step?

Thanks for the analysis and explanation. It may be that all we really need is better comments explaining how it works and then the necessary spot fixes. Since this only affects two binaries it may not be worth doing much restructuring.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f6aa9ef3cd873f0cb097e3621f262c72a3876c7d

commit f6aa9ef3cd873f0cb097e3621f262c72a3876c7d
Author: wez <wez@chromium.org>
Date: Thu Dec 22 21:46:17 2016

Fix conditional for /OPT:ICF so Debug non-component build does not warn.

The condition under which /OPT:ICF was enabled was out-of-sync with the
conditions for enabling incremental linking. Since the two are not
compatible the difference resulted in linker warnings in Debug
non-component builds.

BUG= 621236 ,  659007 

Review-Url: https://codereview.chromium.org/2594153002
Cr-Commit-Position: refs/heads/master@{#440514}

[modify] https://crrev.com/f6aa9ef3cd873f0cb097e3621f262c72a3876c7d/build/config/compiler/BUILD.gn