Support Visual C++ PGO in Ninja generator

Support Visual C++ PGO in Ninja generator

This CL teaches msvs_emulation.py about following settings so that ninja ganarate can handle Progile-Guided Optiomization (PGO) settings for Visual C++.
- VCLinkerTool.LinkTimeCodeGeneration
-- 2: /LTCG:PGINSTRUMENT
-- 3: /LTCG:PGOPTIMIZE
-- 4: /LTCG:PGUPDATE
- VCLinkerTool.ProfileGuidedDatabase

Note that LinkTimeCodeGenerationOption.LinkTimeCodeGenerationOptionUse is partially supported for LTCG (Link-Time Code Generator). This CL also adds unittests for LTCG in addition to unittests for PGO.

BUG=gyp:385
TEST=gyptest.py test/win/gyptest-link-ltcg.py test/win/gyptest-link-pgo.py
R=scottmg@chromium.org

Committed: https://code.google.com/p/gyp/source/detail?r=1803

[yukawa, 2013-11-23 14:55:13]

Hi,

I investigated https://code.google.com/p/gyp/issues/detail?id=343 and concluded
that our implementation is correct. This CL adds unittests which verify our
correctness.

[scottmg, 2013-11-23 17:15:13]

Thanks for doing this!

https://codereview.chromium.org/82703007/diff/1/pylib/gyp/msvs_emulation.py
File pylib/gyp/msvs_emulation.py (right):

https://codereview.chromium.org/82703007/diff/1/pylib/gyp/msvs_emulation.py#n...
pylib/gyp/msvs_emulation.py:490: ld('LinkTimeCodeGeneration',
(Sorry, not at a Windows machine right now to check...) Does the Librarian
section also need to be modified to either 1) handle '1' instead of 'true', or
2) also handle the other LTCG options?

https://codereview.chromium.org/82703007/diff/1/test/win/gyptest-link-ltcg.py
File test/win/gyptest-link-ltcg.py (right):

https://codereview.chromium.org/82703007/diff/1/test/win/gyptest-link-ltcg.py...
test/win/gyptest-link-ltcg.py:22: if test.format == 'ninja':
would it be possible to test some effect, rather than the contents of build
files so that ninja and msvs match? For PGI we could check that a .pgd file is
generated I guess. I'm not sure about PGO and PGU though, perhaps something that
the linker prints to stdout that identifies what it's doing?

[yukawa, 2013-11-25 14:04:12]

Thank you for the review. PTAL?

https://codereview.chromium.org/82703007/diff/1/pylib/gyp/msvs_emulation.py
File pylib/gyp/msvs_emulation.py (right):

https://codereview.chromium.org/82703007/diff/1/pylib/gyp/msvs_emulation.py#n...
pylib/gyp/msvs_emulation.py:490: ld('LinkTimeCodeGeneration',
On 2013/11/23 17:15:13, scottmg wrote:
> (Sorry, not at a Windows machine right now to check...) Does the Librarian
> section also need to be modified to either 1) handle '1' instead of 'true', or
> 2) also handle the other LTCG options?

Yeah, I believe that the Librarian should continue to use boolean since the
underlying MSVS object model uses bool as follows.
| bool LinkTimeCodeGeneration { get; set; } 
http://msdn.microsoft.com/en-us/library/microsoft.visualstudio.vcprojectengin...

Actually Skia is specifying both settings correctly.
https://code.google.com/p/skia/source/browse/trunk/gyp/common_conditions.gypi...

Does this make sense?

https://codereview.chromium.org/82703007/diff/1/test/win/gyptest-link-ltcg.py
File test/win/gyptest-link-ltcg.py (right):

https://codereview.chromium.org/82703007/diff/1/test/win/gyptest-link-ltcg.py...
test/win/gyptest-link-ltcg.py:22: if test.format == 'ninja':
On 2013/11/23 17:15:13, scottmg wrote:
> would it be possible to test some effect, rather than the contents of build
> files so that ninja and msvs match? For PGI we could check that a .pgd file is
> generated I guess. I'm not sure about PGO and PGU though, perhaps something
that
> the linker prints to stdout that identifies what it's doing?

OK.  Added some working samples as pgo.gyp.  PTAL?

For PGI, this CL tests if *.pgd is generated.
For PGO, this CL tests if 'profiled functions' appears in the stdout during the
build.
For PGU, this CL tests the profile guided build succeeds or not even when some
*.cc is newer than *.pgd file. (This is what PGU is designed for)

The followings are notes about what I encountered. Hope this helps me again when
I use PGO in my project.

Firstly, Under some environment, PGO may not be supported. I simply skipped the
real test in such cases.

Secondary, executables build with PGI depend on some special DLLs installed
under '$(VCInstallDir)bin'.  To work around this, I added 'update_pgd.py' to set
up PATH environment variable before executing the instrumented binary.

Finally, executables build with PGI generate a *.pgc file where the profile data
is stored.  The problem here is that its file name is not deterministic.  This
makes it difficult to describe a correct dependency as a gyp 'action'.  In this
patch, I used my 'update_pgd.py' too to kick pgomgr.exe just after execution of
target binary so that merge all *.pgc data will be merged into single *.pgd
file, whose name is under our control.  This problem might also be solved by
copying the target executable into a separated temporary directory, then after
execution, rename *.pgc found in the directory to some deterministic one.

Anyway my lessons learned from this trial is that cyclic procedure implied from
profile guided build flow don't go along well with typical build systems, which
expect the dependency graph is directed and acyclic in general.  Build is hard.

[scottmg, 2013-11-25 15:13:39]

Great work! Sorry it got so complex to test.

I agree the build flow is ugly.
http://msdn.microsoft.com/en-us/library/xbf3tbeh.aspx suggests that it's
possible to use /OUT with the /LTCG:PGO step, perhaps we could use that to
maintain the DAG build structure. (I haven't tested that to know if it causes
other problems though.)

Otherwise, maybe at some point should investigate having a gyp option that
builds a structure similar to pgo.gyp automatically.

LGTM

https://codereview.chromium.org/82703007/diff/960001/test/win/linker-flags/pg...
File test/win/linker-flags/pgo.gyp (right):

https://codereview.chromium.org/82703007/diff/960001/test/win/linker-flags/pg...
test/win/linker-flags/pgo.gyp:14: # 'executable' targets should share the code
itself so taht profile data
nit; "taht"

[yukawa, 2013-12-03 16:10:41]

On 2013/11/25 15:13:39, scottmg wrote:
> http://msdn.microsoft.com/en-us/library/xbf3tbeh.aspx suggests that it's
> possible to use /OUT with the /LTCG:PGO step, perhaps we could use that to
> maintain the DAG build structure. (I haven't tested that to know if it causes
> other problems though.)

Thank you for the info!  Yeah, actually my patch is doing a similar things by
having different 'target_name' for each PGI/PGO/PGU build.

Anyway, now all the tests become green. I'm glad that I can finish this task!

https://codereview.chromium.org/82703007/diff/960001/test/win/linker-flags/pg...
File test/win/linker-flags/pgo.gyp (right):

https://codereview.chromium.org/82703007/diff/960001/test/win/linker-flags/pg...
test/win/linker-flags/pgo.gyp:14: # 'executable' targets should share the code
itself so taht profile data
On 2013/11/25 15:13:39, scottmg wrote:
> nit; "taht"

Done.

[yukawa, 2013-12-03 16:12:12]

Committed patchset #4 manually as r1803.