New issue
Advanced search Search tips

Issue 627216 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 634788
issue 637456

Blocking:
issue 619446
issue 636468



Sign in to add a comment

Upgrade to Update 3 of VS 2015

Project Member Reported by brucedaw...@chromium.org, Jul 11 2016

Issue description

Update 3 of VS 2015 is available. It fixes almost 200 compiler bugs since Update 2, removes the telemetry calls which worry some people, and has some improvements to the optimizer.

The new optimizer is described here:
https://blogs.msdn.microsoft.com/vcblog/2016/05/04/new-code-optimizer/

Compiler improvements:
https://blogs.msdn.microsoft.com/vcblog/2016/06/07/compiler-improvements-in-vs-2015-update-3-rc/

Conformance improvements are listed here:
https://msdn.microsoft.com/library/mt723604.aspx#VS_Update3

Some specific bugs that Chrome has hit include:

Update 2 hits an internal compiler error when building Chrome with /analyze, which has rendered /analyze unusable:
https://bugs.chromium.org/p/chromium/issues/detail?id=608381
https://connect.microsoft.com/VisualStudio/feedback/details/2659765/internal-compiler-error-when-building-chromium-with-analyze

Incremental linking crash:
https://bugs.chromium.org/p/chromium/issues/detail?id=482671
https://connect.microsoft.com/VisualStudio/feedback/details/1289562

Telemetry calls are removed in Update 3:
https://bugs.chromium.org/p/chromium/issues/detail?id=619446

 
Blocking: 619446
Note that Microsoft appears to have republished the 1511 version of the Windows 10 SDK so that what files you get depends on when you installed it. Reinstalling does not correct this. This makes getting a reproducible SDK seemingly impossible - at least three variants have been reported depending on the exact steps taken.
When upgrading don't forget to update the version check:

See crrev.com/1849423002 for the previous check.
If you find that vs2015.3.exe refuses to update your local VS installation to Update 3 (happened for me as well) you can follow the update instructions here: http://stackoverflow.com/questions/38076943/visual-studio-2015-update-3-not-installing-no-error-or-feedback
@brucedawson: Using a local installation of Update 3 (DEPOT_TOOLS_WIN_TOOLCHAIN=0) it looks like GN is selecting the 10.0.14393 SDK by default when writing the environment.* files. Is there a way to tell GN to use the supported 10.0.10586 SDK version instead?
@comment#5: Looks like we can resolve this by passing the SDK version as the 2nd argument to vcvarsall.bat in build/toolchain/win/setup_toolchain.py [1]. The script will then return environment variables configured to use that version. For example:

"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" amd64 10.0.10586.0

We'd probably also want to add a new GN argument in build/config/win/visual_studio_version.gni for populating this value, and default it to the currently supported SDK.

[1] https://cs.chromium.org/chromium/src/build/toolchain/win/setup_toolchain.py?dr=C&q=setup_toolchain.py+vcvarsall.bat&sq=package:chromium&l=126
Blockedon: 634788
Added issue #634788 for comment #5/6.
Cc: scottmg@chromium.org
Per https://bugs.chromium.org/p/chromium/issues/detail?id=636468 incremental linking is very broken for common cases and appears to be fixed in Update 3 final. So it would be great to get U3 packaged and rolled out.
Blocking: 636468
While there are compiler fixes in Update 3 there are also regressions, so we will have to proceed with care.

https://connect.microsoft.com/VisualStudio/feedback/details/3001148/wrong-code-generation-for-mm-insert-ps-x64-optimized


Regarding the SDK version, I believe that GN (like GYP before it) gets the environment variable settings by running vcvarsall.bat. At some point we added specific include/lib paths at the front in order to specify what SDK version we built with, but those caused their own problems (very long environment variable paths, complicated support for non-standard install locations), however we may need to bring them back.

In particular, there is a bluetooth callback declaration which is different in the 14393 SDK - a 'CALLBACK' modifier is needed in order to compile with 14393. If we upgrade Chrome to 14393 then everybody building with a locally installed SDK needs to use 14393, and prior to our upgrading Chrome they need to use 10586. Here is the diff to support the change to 14393:

diff --git a/device/bluetooth/bluetooth_task_manager_win.cc b/device/bluetooth/bluetooth_task_manager_win.cc
index e792947..62864f8 100644
--- a/device/bluetooth/bluetooth_task_manager_win.cc
+++ b/device/bluetooth/bluetooth_task_manager_win.cc
@@ -154,7 +154,7 @@ base::Lock g_characteristic_value_changed_registrations_lock;

 // Function to be registered to OS to monitor Bluetooth LE GATT event. It is
 // invoked in BluetoothApis.dll thread.
-void OnGetGattEventWin(BTH_LE_GATT_EVENT_TYPE type,
+void CALLBACK OnGetGattEventWin(BTH_LE_GATT_EVENT_TYPE type,
                        PVOID event_parameter,
                        PVOID context) {
   if (type != CharacteristicValueChangedEvent) {

Blockedon: 637456
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 19 2016

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

commit 0ea25a46ed83e529d928190cbf3259964acd3ad1
Author: brucedawson <brucedawson@chromium.org>
Date: Fri Aug 19 23:16:58 2016

Allow testing of Update 3 final

Upgrading to Update 3 is hitting a few roadblocks, which is preventing
Windows developers from getting its fast-incremental-linking goodness.
This change lets developers opt-in to using it by going:

    set DEPOT_TOOLS_WIN_TOOLCHAIN_PRERELEASE=1
    gclient runhooks

If the environment variable is not set then this has no effect.

BUG= 627216 

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

[modify] https://crrev.com/0ea25a46ed83e529d928190cbf3259964acd3ad1/build/vs_toolchain.py

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 24 2016

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

commit 109db8b6fcdb52a55e40c44128a9cf36c00a5649
Author: thakis <thakis@chromium.org>
Date: Wed Aug 24 15:57:13 2016

clang/win: Make builds work with MSVC2015 update 3.

The midl compiler now produces slightly questionable code that clang-cl warns
on.  The warning is no longer emitted in upstream clang-cl, but until we roll
that in, suppress it so that people who locally use update 3 are able to build
with clang.

BUG= 637456 ,  627216 ,  640000 
NOTRY=true

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

[modify] https://crrev.com/109db8b6fcdb52a55e40c44128a9cf36c00a5649/build/config/win/BUILD.gn

Comment 14 Deleted

The attached file gives an internal compiler error when compiled with Update 3 of VS 2015, but not with Update 2. All you have to do is go:

    cl /c /Ox Skia_VS2015_Update3_ICE.cpp

Compiler version is Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24213.1 for x86
Skia_VS2015_Update3_ICE.cpp
1.8 MB View Download
The Internal Compiler Error bug mentioned in the previous comment has been filed as:
https://connect.microsoft.com/VisualStudio/feedback/details/3100520/internal-compiler-error
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 8 2016

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

commit 02343c661ee0482cb73ad0cf861999f2c57eeb11
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Sep 08 01:53:00 2016

Switch to VS 2015 Update 3

VS 2015 Update 3 offers numerous improvements such as improved
language conformance, fixes to avoid linker crashes, and
improved code-gen for faster performance. Update 3 also resolves
some problems with incremental linking silently falling back to
a full link.

It also fixes an internal compiler error in /analyze builds.

The landmines change is needed because otherwise things like the
out\Release\cdb directory do not get regenerated.

The process for creating this package is:
- Create a clean Windows VM running Windows Server 2012 R2 (en_windows_server_2012_r2_vl_with_update_x64_dvd_6052766.iso, SHA1: 247EAEE5628850A41F0C51471656AAFB2492EA08, Standard Edition)
- Install depot tools, run ‘gclient’, and add depot_tools to the path
- Install VS 2015 Professional Update 3 - must have been installed after July 12th to get the latest fixes - with these settings and nothing else:
 * Visual C++ and make sure the three nodes underneath are also selected
 * Under Universal Windows App Development Tools make sure the "Tools (1.4.1) and Windows 10 SDK (10.0.14393)" and the "Windows 10 SDK (10.0.10586)" nodes are selected and nothing else
- Copy the *Debugger and Tools*.msi installers from another machine and run them. The 10.0.14393 versions (Anniversary Edition) should be used.
- Then run:
    python depot_tools\win_toolchain\package_from_installed.py 2015

It is also possible to package the 14393 SDK by running this command,
but that change is being saved for later:
    python depot_tools\win_toolchain\package_from_installed.py 2015 -w 10.0.14393.0

BUG= 627216 , 636468 , 427616 , 637456 

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

[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/get_landmines.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/vs_toolchain.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/win/copy_cdb_to_output.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/tools/perf/chrome_telemetry_build/BUILD.gn

Project Member

Comment 18 by bugdroid1@chromium.org, Sep 8 2016

Labels: merge-merged-2854
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02343c661ee0482cb73ad0cf861999f2c57eeb11

commit 02343c661ee0482cb73ad0cf861999f2c57eeb11
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Sep 08 01:53:00 2016

Switch to VS 2015 Update 3

VS 2015 Update 3 offers numerous improvements such as improved
language conformance, fixes to avoid linker crashes, and
improved code-gen for faster performance. Update 3 also resolves
some problems with incremental linking silently falling back to
a full link.

It also fixes an internal compiler error in /analyze builds.

The landmines change is needed because otherwise things like the
out\Release\cdb directory do not get regenerated.

The process for creating this package is:
- Create a clean Windows VM running Windows Server 2012 R2 (en_windows_server_2012_r2_vl_with_update_x64_dvd_6052766.iso, SHA1: 247EAEE5628850A41F0C51471656AAFB2492EA08, Standard Edition)
- Install depot tools, run ‘gclient’, and add depot_tools to the path
- Install VS 2015 Professional Update 3 - must have been installed after July 12th to get the latest fixes - with these settings and nothing else:
 * Visual C++ and make sure the three nodes underneath are also selected
 * Under Universal Windows App Development Tools make sure the "Tools (1.4.1) and Windows 10 SDK (10.0.14393)" and the "Windows 10 SDK (10.0.10586)" nodes are selected and nothing else
- Copy the *Debugger and Tools*.msi installers from another machine and run them. The 10.0.14393 versions (Anniversary Edition) should be used.
- Then run:
    python depot_tools\win_toolchain\package_from_installed.py 2015

It is also possible to package the 14393 SDK by running this command,
but that change is being saved for later:
    python depot_tools\win_toolchain\package_from_installed.py 2015 -w 10.0.14393.0

BUG= 627216 , 636468 , 427616 , 637456 

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

[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/get_landmines.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/vs_toolchain.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/win/copy_cdb_to_output.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/tools/perf/chrome_telemetry_build/BUILD.gn

Status: Fixed (was: Assigned)
Looking good so far. Closing as fixed.

Sign in to add a comment