New issue
Advanced search Search tips

Issue 759402 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 727437

Blocking:
issue 683729



Sign in to add a comment

dEQP functional_shaders_matrix_matrixcompmult_dynamic* tests failing on Win x64

Project Member Reported by kbr@chromium.org, Aug 27 2017

Issue description

As of this build:
https://luci-milo.appspot.com/buildbot/chromium.gpu.fyi/Win7%20x64%20dEQP%20Release%20%28NVIDIA%29/460

the following drawElements (dEQP) tests are failing:

dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_mediump_mat4_mat4_fragment
dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_highp_mat4_mat4_vertex
dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_lowp_mat4_mat4_fragment
dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_mediump_mat4_mat4_vertex
dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_lowp_mat4_mat4_vertex
dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_highp_mat4_mat4_fragment

The logs aren't that useful:

[ RUN      ] dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_highp_mat4_mat4_fragment
dEQP-GLES2.functional.shaders.matrix.matrixcompmult.dynamic_highp_mat4_mat4_fragment
../../third_party/angle/src/tests/deqp_support/angle_deqp_gtest.cpp(289): error: Value of: result
  Actual: false
Expected: true
[  FAILED  ] dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_highp_mat4_mat4_fragment, where GetParam() = 6304 (128 ms)

Bruce, I think this is caused by the switch to VC++ 2017 in https://chromium.googlesource.com/chromium/src/+/ec9a1e6ce2708f93ea08f82dd76a1f556310af91 . I remember reading you were going to roll back that change Monday, but regardless could you keep this on the radar for making this roll permanent? Thanks.

 
That's a good reminder that I should revert now - we already have a canary (62.0.3197.0) and confirmation that ToT clang is working.

I will monitor the test status as the revert lands, and investigate. Could be another compiler bug or compiler dependency.

Build 506 went green and the only change in that build was the revert back to VC++ 2015. So, confirmed that the failures are correlated with VC++ 2017.
Can you explain how to run these tests? I've tried building angle_deqp_gles2_tests and angle_deqp_gles2_d3d11_tests from my Chromium enlistments and they are both unknown targets. I suspect some magic in running these locally also - any pointers would be appreciated.
Because these tests use exceptions, we had to put them behind a build flag. Set build_angle_deqp_tests = true in your GN configuration and you should see them!
You need build_angle_deqp_tests=true to build these. They are regular gtest harnesses so you can use --gtest_filter=dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_highp_mat4_mat4_fragment to target that specific failure.
I've reproduced the issue locally (just set GYP_MSVS_VERSION=2017, gclient runhooks, gn gen, and run the tests) and confirmed that they only happen with VS 2017 on release builds (but we knew that already). I'm stepping through the code but I'm having trouble finding where things have gone awry.

Any tips on debugging failures in these tests? Is there a way to dump the reference and result images to see what sort of differences are showing up? The difference calculated by fuzzyCompare is 322 which is pretty huge given that the threshold is 0.05. What is the range of results possible for fuzzyCompare? I'm wondering if the bug might be in there rather than in the code that generates the image.

I've attached a screenshot showing a memory-window view of the beginning of the reference image (left) and the result image (right). The right-most channel (R?) is slightly off but the middle channel (B?) is totally wrong - it's a constant '01' in the result image.

I don't have any applicable strategies for how to proceed. Ideas? I'm using these gn args:

is_clang = false
is_component_build = false
is_debug = false
target_cpu = "x64"
enable_nacl = false
treat_warnings_as_errors = false

use_goma = true
is_win_fastlink = true
symbol_level = 2
build_angle_deqp_tests = true

I then build angle_deqp_gles2_tests.exe and run it like this:

out\release_64\angle_deqp_gles2_tests.exe --gtest_filter=dEQP_GLES2.Default/functional_shaders_matrix_matrixcompmult_dynamic_highp_mat4_mat4_fragment

I've tried selectively disabling optimizations to make debugging a bit easier, and to see if doing so would make the bug go away (since debug builds don't show the problem) but no luck.

I did find that changing the config("optimize") in build\config\compiler\BUILD.gn to disable optimizations avoids the problem, which proves that it is optimizations (not some other release/debug difference) that triggers the bug, but there are 1385 build steps that run when making that change so it doesn't get me much closer to figuring out which ones are the problem.

ReferenceAndResult.PNG
34.1 KB View Download
After some off-bug discussions I checked to see whether the reference image was the erroneous one and it was. A bit of experimentation in the generation code led to this minimal change to work around the problem:

third_party\deqp\src\modules\gles2\functional>git diff
diff --git a/modules/gles2/functional/es2fShaderMatrixTests.cpp b/modules/gles2/functional/es2fShaderMatrixTests.cpp
index 4b6a8fd3..62cca317 100644
--- a/modules/gles2/functional/es2fShaderMatrixTests.cpp
+++ b/modules/gles2/functional/es2fShaderMatrixTests.cpp
@@ -1084,6 +1084,8 @@ ShaderMatrixTests::~ShaderMatrixTests (void)
 {
 }

+#pragma optimize("", off)
+
 void ShaderMatrixTests::init (void)
 {
        static const struct


That is, disabling optimizations for that one function avoids the test failure. So, we now have a fallback plan, if needed, and I can now more easily investigate what is going on.

Correction - disabling optimizations at the end of the file avoids the test failure. That is weird is because #pragma optimize is documented as only affecting functions defined after it, and there are none, and yet that works. So we have an even more minimal workaround, and a mystery.
That's really weird. Splitting the file in two to have the pragma apply to a subset of the code looks hard. Maybe we could diff the size of each function with/without the pragma and it would highlight what changes?
I'm going to try stepping through the code in a debug build enough to be familiar with how it is supposed to work and then repeat the process in a release build. I'm hopeful that I've isolated the problematic code to a small enough region for that to work, but ???
I managed to isolate the issue. I don't know what triggers it but I have now identified which function is generating bad code and the code is obviously bad (easily verified through inspection) which makes reporting it to Microsoft easy. They can build the associated file from a Chrome repo but I will make it easier by creating a preprocessed file which they can compile and then see the error. The problematic function (from the Visual Studio call stack) is:

> angle_deqp_libgles2.dll!deqp::gles2::Functional::MatrixCaseUtils::matrixCompMult<float,4,4>(const tcu::Matrix<float,4,4> & a, const tcu::Matrix<float,4,4> & b)

The source code is shown below and it clearly consists of two nested loops each of which should execute four iterations, for a total of sixteen multiply operations:

template <typename T, int Rows, int Cols>
tcu::Matrix<T, Rows, Cols> matrixCompMult (const tcu::Matrix<T, Rows, Cols>& a, const tcu::Matrix<T, Rows, Cols>& b)
{
	tcu::Matrix<T, Rows, Cols> retVal;

	for (int r = 0; r < Rows; ++r)
		for (int c = 0; c < Cols; ++c)
			retVal(r,c) = a(r,c) * b(r, c);

	return retVal;
}

The generated code is shown below. Note that r8 is preserved, then initialized with 4, then decremented by 1, and this is followed by a jne. Inspection shows that the function contains no other branch instructions so the code from *356 to *3b3 executes four times. The whole function contains just one mulps instruction, in the loop. That instruction is executed four times. I don't know if this represents incorrect unwinding of the loop or ???

template <typename T, int Rows, int Cols>
tcu::Matrix<T, Rows, Cols> matrixCompMult (const tcu::Matrix<T, Rows, Cols>& a, const tcu::Matrix<T, Rows, Cols>& b)
{
00007FFE28EB5324  mov         qword ptr [rsp+8],rbx  
00007FFE28EB5329  mov         qword ptr [rsp+10h],rsi  
00007FFE28EB532E  push        rdi  
00007FFE28EB532F  sub         rsp,20h  
00007FFE28EB5333  mov         rdi,r8  
00007FFE28EB5336  mov         rbx,rdx  
00007FFE28EB5339  mov         rsi,rcx  
	tcu::Matrix<T, Rows, Cols> retVal;
00007FFE28EB533C  call        tcu::Matrix<float,4,4>::Matrix<float,4,4> (07FFE28EB5FA4h)  
00007FFE28EB5341  lea         rdx,[rdi+10h]  
00007FFE28EB5345  mov         r8d,4  
00007FFE28EB534B  sub         rdi,rbx  
00007FFE28EB534E  lea         rax,[rbx+8]  
00007FFE28EB5352  lea         rcx,[rsi+8]  

	for (int r = 0; r < Rows; ++r)
		for (int c = 0; c < Cols; ++c)
			retVal(r,c) = a(r,c) * b(r, c);
00007FFE28EB5356  movss       xmm1,dword ptr [rdi+rax+4]  
00007FFE28EB535C  lea         rcx,[rcx+10h]  
00007FFE28EB5360  movss       xmm0,dword ptr [rdi+rax]  
00007FFE28EB5365  lea         rdx,[rdx+10h]  
00007FFE28EB5369  movss       xmm2,dword ptr [rdi+rax-4]  
00007FFE28EB536F  movss       xmm4,dword ptr [rdi+rax-8]  
00007FFE28EB5375  movss       xmm3,dword ptr [rax-8]  
00007FFE28EB537A  unpcklps    xmm4,xmm0  
00007FFE28EB537D  movss       xmm0,dword ptr [rax]  
00007FFE28EB5381  lea         rax,[rax+10h]  

	for (int r = 0; r < Rows; ++r)
		for (int c = 0; c < Cols; ++c)
			retVal(r,c) = a(r,c) * b(r, c);
00007FFE28EB5385  unpcklps    xmm2,xmm1  
00007FFE28EB5388  movss       xmm1,dword ptr [rax-0Ch]  
00007FFE28EB538D  unpcklps    xmm4,xmm2  
00007FFE28EB5390  movss       xmm2,dword ptr [rax-14h]  
00007FFE28EB5395  unpcklps    xmm2,xmm1  
00007FFE28EB5398  unpcklps    xmm3,xmm0  
00007FFE28EB539B  unpcklps    xmm3,xmm2  
00007FFE28EB539E  mulps       xmm4,xmm3  
00007FFE28EB53A1  movss       dword ptr [rcx-18h],xmm4  
00007FFE28EB53A6  shufps      xmm4,xmm4,0E5h  
00007FFE28EB53AA  movsd       mmword ptr [rcx-14h],xmm4  
00007FFE28EB53AF  sub         r8,1  
00007FFE28EB53B3  jne         deqp::gles2::Functional::MatrixCaseUtils::matrixCompMult<float,4,4>+32h (07FFE28EB5356h)  

	return retVal;
}
00007FFE28EB53B5  mov         rbx,qword ptr [rsp+30h]  
00007FFE28EB53BA  mov         rax,rsi  
00007FFE28EB53BD  mov         rsi,qword ptr [rsp+38h]  
00007FFE28EB53C2  add         rsp,20h  
00007FFE28EB53C6  pop         rdi  
00007FFE28EB53C7  ret  

Add VS 2017 64-bit compiler to the path:

> "C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Auxiliary\Build\vcvarsall.bat" amd64

> where cl
C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.11.25503\bin\HostX64\x64\cl.exe

> cl
Microsoft (R) C/C++ Optimizing Compiler Version 19.11.25507.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

usage: cl [ option... ] filename... [ /link linkoption... ]


Then compile the attached preprocessed repro:

> cl /c es2fShaderMatrixTests.cpp /O1 /Ob2 /Oi /EHsc /FAcs

Search the resulting .cod file for this string:

    matrixCompMult<float,4,4>, COMDAT

After deleting everything outside of that COMDAT you are left with the attached .cod file. Searching on " j" shows one branch instruction based on decrementing r8. Searching for "r8" shows that r8d is initialized to 4. Searching on "mul" shows that there is just one multiply instruction. Since the function is instantiated with Rows and Cols both equal to 4 this is clearly wrong.

The Visual Studio bug is filed at https://developercommunity.visualstudio.com/content/problem/104890/code-gen-bug-in-1532-x64-compiler.html

es2fShaderMatrixTests.cpp
1.4 MB View Download
es2fShaderMatrixTests.cod
2.8 KB Download
I couldn't find instructions on how to contribute to deqp - would somebody else like to land a patch to work around this (presumed verified) compiler bug? The patch looks like this:

commit 4a6e06d79f93fba70118e07116e0d3c324934b28 (HEAD -> workaround)
Author: Bruce Dawson <brucedawson@chromium.org>
Date:   Wed Aug 30 17:05:14 2017 -0700

    Work around VC++ 2017 compiler bug

    A code-gen bug was found in 15.3.2 (VC++ 2017 Update 3.2) that causes
    test failures due to one loop being skipped. Attempts to disable
    optimizations for just the affected function failed so optimizations
    must be disabled at the end of the source file, which seems to cause all
    template functions and compiler generated functions to have
    optimizations disabled. A VS bug has been filed and cross-linked.

    BUG= 759402 

diff --git a/modules/gles2/functional/es2fShaderMatrixTests.cpp b/modules/gles2/functional/es2fShaderMatrixTests.cpp
index 4b6a8fd3..96a6afd6 100644
--- a/modules/gles2/functional/es2fShaderMatrixTests.cpp
+++ b/modules/gles2/functional/es2fShaderMatrixTests.cpp
@@ -1221,3 +1221,9 @@ void ShaderMatrixTests::init (void)
 } // Functional
 } // gles2
 } // deqp
+
+#if defined(_MSC_VER) && _MSC_FULL_VER == 191125507
+// Work around  crbug.com/759402  which is a code-gen bug in VC++ 2015, version
+// 15.3.2.
+#pragma optimize("", off)
+#endif



As the comment says I tried to find a more scoped change but failed. If you have any other ideas let me know. I filed three bugs against VS - one for the code-gen error, one for an internal-compiler-error I hit while trying to disable optimizations on just one template function, and one for the fact that you apparently can't disable optimizations for just one template function.

s/VC++ 2015/VC++2017/ in the comment (?)
> s/VC++ 2015/VC++2017/ in the comment (?)

Ah - yes. This is a VC++ 2017 bug. My guess is it only happens with Update 3, maybe only update 3.2, but ???
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/fb2a871c0a907b5a6dc81533ce0636550f881bab

commit fb2a871c0a907b5a6dc81533ce0636550f881bab
Author: Geoff Lang <geofflang@chromium.org>
Date: Fri Sep 01 16:12:01 2017

Work around VC++ 2017 compiler bug

A code-gen bug was found in 15.3.2 (VC++ 2017 Update 3.2) that causes
test failures due to one loop being skipped. Attempts to disable
optimizations for just the affected function failed so optimizations
must be disabled at the end of the source file, which seems to cause all
template functions and compiler generated functions to have
optimizations disabled. A VS bug has been filed and cross-linked.

BUG= 759402 

Change-Id: Ida765a47234a63bad48e6a4e910f3b82919d6be9
Reviewed-on: https://chromium-review.googlesource.com/647313
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>

[add] https://crrev.com/fb2a871c0a907b5a6dc81533ce0636550f881bab/src/tests/deqp_support/es2fShaderMatrixTests.cpp
[modify] https://crrev.com/fb2a871c0a907b5a6dc81533ce0636550f881bab/src/tests/deqp.gypi

I confirmed that a modifying DEPS to pull in Angle from fb2a871c0a907b5a6dc81533ce0636550f881bab fixes the test failures.

I'll close this when the DEPS roll happens. Microsoft is investigating the bug on their side.
Project Member

Comment 19 by bugdroid1@chromium.org, Sep 1 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/ebef5ff355b7c14bda99c9b6a97555d1d55c4da3

commit ebef5ff355b7c14bda99c9b6a97555d1d55c4da3
Author: angle-deps-roller@chromium.org <angle-deps-roller@chromium.org>
Date: Fri Sep 01 18:48:59 2017

Roll skia/third_party/externals/angle2/ f00f7ffed..fb2a871c0 (1 commit)

https://chromium.googlesource.com/angle/angle.git/+log/f00f7ffed59e..fb2a871c0a90

$ git log f00f7ffed..fb2a871c0 --date=short --no-merges --format='%ad %ae %s'
2017-09-01 geofflang Work around VC++ 2017 compiler bug

Created with:
  roll-dep skia/third_party/externals/angle2
BUG= 759402 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=skia.primary:Perf-Win10-MSVC-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-ANGLE,Perf-Win10-MSVC-NUC5i7RYH-GPU-IntelIris6100-x86_64-Debug-ANGLE,Perf-Win10-MSVC-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-ANGLE,Perf-Win10-MSVC-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-ANGLE,Perf-Win10-MSVC-ShuttleC-GPU-GTX960-x86_64-Debug-ANGLE,Perf-Win10-MSVC-ZBOX-GPU-GTX1070-x86_64-Debug-ANGLE,Test-Win10-MSVC-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-ANGLE,Test-Win10-MSVC-NUC5i7RYH-GPU-IntelIris6100-x86_64-Debug-ANGLE,Test-Win10-MSVC-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-ANGLE,Test-Win10-MSVC-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-ANGLE,Test-Win10-MSVC-ShuttleC-GPU-GTX960-x86_64-Debug-ANGLE,Test-Win10-MSVC-ZBOX-GPU-GTX1070-x86_64-Debug-ANGLE
TBR=djsollen@google.com

Change-Id: I2888ce8153e5ea1a5e738ae7b7ab280bf962bc56
Reviewed-on: https://skia-review.googlesource.com/42041
Reviewed-by: angle-deps-roller . <angle-deps-roller@chromium.org>
Commit-Queue: angle-deps-roller . <angle-deps-roller@chromium.org>

[modify] https://crrev.com/ebef5ff355b7c14bda99c9b6a97555d1d55c4da3/DEPS

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 2 2017

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

commit 812e13998dd866d13fcd7cb2834e88c845b6939c
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Fri Sep 01 23:59:46 2017

Roll src/third_party/skia/ dbb24efcc..de7f91b29 (9 commits)

https://skia.googlesource.com/skia.git/+log/dbb24efcc5f2..de7f91b29fd8

$ git log dbb24efcc..de7f91b29 --date=short --no-merges --format='%ad %ae %s'
2017-08-18 scroggo Remove SK_LEGACY_FRAME_INFO_ALPHA_TYPE
2017-09-01 caryclark bookmaker spelling with fixed linux build
2017-09-01 liyuqian Use SkSTArenaAlloc to manage SkCoverageDeltaMask's stack memory
2017-09-01 caryclark generated SkCanvas.h
2017-09-01 caryclark Revert "wip spelling mania"
2017-09-01 liyuqian Remove unused drawPath
2017-09-01 angle-deps-roller Roll skia/third_party/externals/angle2/ f00f7ffed..fb2a871c0 (1 commit)
2017-09-01 liyuqian Let SampleApp provide the thread pool
2017-09-01 caryclark wip spelling mania

Created with:
  roll-dep src/third_party/skia
BUG= 759402 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=djsollen@chromium.org

Change-Id: Id15b8cdcb01f212ac4f1b8a475be31ae9dd75395
Reviewed-on: https://chromium-review.googlesource.com/648432
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499348}
[modify] https://crrev.com/812e13998dd866d13fcd7cb2834e88c845b6939c/DEPS

Project Member

Comment 21 by bugdroid1@chromium.org, Sep 5 2017

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

commit 7dcbe4dd3b6868fdd77f7bc04d47c6c2341d407a
Author: Jamie Madill <jmadill@chromium.org>
Date: Tue Sep 05 06:43:44 2017

Roll ANGLE 855d964..72b4e1e

https://chromium.googlesource.com/angle/angle.git/+log/855d964..72b4e1e

BUG= 759402 , 602737 

TBR=geofflang@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Change-Id: If5c6dc88ce9dbb9c790f5b59c92bef2cf6ede3d8
Reviewed-on: https://chromium-review.googlesource.com/650007
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499578}
[modify] https://crrev.com/7dcbe4dd3b6868fdd77f7bc04d47c6c2341d407a/DEPS

Comment 22 by kbr@chromium.org, Sep 5 2017

Thanks Bruce and ANGLE folks for tracking down this nasty compiler bug!

Project Member

Comment 23 by bugdroid1@chromium.org, Sep 5 2017

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

commit 27219f480ef91d941fdf51572de9f2575119a9e4
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Tue Sep 05 19:01:24 2017

Use constepxr on default_dispatch to avoid crashes

In obscure and unusual configurations (disabling optimizations in
release builds in order to track down a VC++ 2017 compiler bug) the
AllocatorDispatch::default_dispatch object is not initialized at compile
time. This means that when very early allocations (before main/WinMain)
happen they cause nullptr dereferences. Using constexpr guarantees
(well, modulo weirdness in  https://crbug.com/660967  and
https://crbug.com/660828) this won't happen.

Unfortunately VC++ 2015 doesn't support tagging a static const member
variable with constexpr which is why this CL is a bit messy.

BUG= 759402 

Change-Id: I77393ebb2a77c2ef7971b37f13d23561a3b02f07
Reviewed-on: https://chromium-review.googlesource.com/639471
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499694}
[modify] https://crrev.com/27219f480ef91d941fdf51572de9f2575119a9e4/base/allocator/allocator_shim_default_dispatch_to_winheap.cc

Re #14 -- I'll look into getting the dEQP workaround upstreamed into the right places-- thanks for the patch! If it's useful in the future, GLES patches should usually go to dEQP in AOSP master: https://android.googlesource.com/platform/external/deqp/+/master (and AOSP gerrit is at: https://android-review.googlesource.com -- the Android contribution guide at https://source.android.com/source/submit-patches will work, but I think I usually just git push to refs/for/blahblah like I do for ANGLE).
Project Member

Comment 25 by bugdroid1@chromium.org, Sep 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/634909433d563285106fa9c48cf98581cb8b657b

commit 634909433d563285106fa9c48cf98581cb8b657b
Author: Jamie Madill <jmadill@chromium.org>
Date: Sat Sep 16 03:41:14 2017

Work around VS 2017 compiler bug in dEQP.

We should upstream this fix to dEQP once we have the chance.

BUG= chromium:759402 

Change-Id: I64e5df9bc6552e6fabe2b4b60c877fa30fd4c1f2
Reviewed-on: https://chromium-review.googlesource.com/670101
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

[add] https://crrev.com/634909433d563285106fa9c48cf98581cb8b657b/src/tests/deqp_support/es3fShaderMatrixTests.cpp
[modify] https://crrev.com/634909433d563285106fa9c48cf98581cb8b657b/src/tests/deqp_support/es2fShaderMatrixTests.cpp
[modify] https://crrev.com/634909433d563285106fa9c48cf98581cb8b657b/src/tests/deqp.gypi

Project Member

Comment 26 by bugdroid1@chromium.org, Sep 16 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/b8a7c8cd1c410465806deda32181618b04e8517d

commit b8a7c8cd1c410465806deda32181618b04e8517d
Author: angle-deps-roller@chromium.org <angle-deps-roller@chromium.org>
Date: Sat Sep 16 05:49:13 2017

Roll skia/third_party/externals/angle2/ 806cda936..634909433 (1 commit)

https://chromium.googlesource.com/angle/angle.git/+log/806cda936c62..634909433d56

$ git log 806cda936..634909433 --date=short --no-merges --format='%ad %ae %s'
2017-09-15 jmadill Work around VS 2017 compiler bug in dEQP.

Created with:
  roll-dep skia/third_party/externals/angle2
BUG= 759402 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=skia.primary:Perf-Win10-MSVC-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-ANGLE,Perf-Win10-MSVC-Golo-GPU-QuadroP400-x86_64-Debug-ANGLE,Perf-Win10-MSVC-NUC5i7RYH-GPU-IntelIris6100-x86_64-Debug-ANGLE,Perf-Win10-MSVC-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-ANGLE,Perf-Win10-MSVC-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-ANGLE,Perf-Win10-MSVC-ShuttleC-GPU-GTX960-x86_64-Debug-ANGLE,Test-Win10-MSVC-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-ANGLE,Test-Win10-MSVC-Golo-GPU-QuadroP400-x86_64-Debug-ANGLE,Test-Win10-MSVC-NUC5i7RYH-GPU-IntelIris6100-x86_64-Debug-ANGLE,Test-Win10-MSVC-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-ANGLE,Test-Win10-MSVC-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-ANGLE,Test-Win10-MSVC-ShuttleC-GPU-GTX960-x86_64-Debug-ANGLE
TBR=stephana@google.com

Change-Id: Ic9415182f1fdafa6ad9942a87948cf2f90b2ea94
Reviewed-on: https://skia-review.googlesource.com/47641
Reviewed-by: angle-deps-roller . <angle-deps-roller@chromium.org>
Commit-Queue: angle-deps-roller . <angle-deps-roller@chromium.org>

[modify] https://crrev.com/b8a7c8cd1c410465806deda32181618b04e8517d/DEPS

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 16 2017

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

commit f49754fb9b9d92639cddea6638b344c507bc5aed
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Sat Sep 16 07:40:19 2017

Roll src/third_party/skia/ f757ae6aa..b8a7c8cd1 (1 commit)

https://skia.googlesource.com/skia.git/+log/f757ae6aa74d..b8a7c8cd1c41

$ git log f757ae6aa..b8a7c8cd1 --date=short --no-merges --format='%ad %ae %s'
2017-09-16 angle-deps-roller Roll skia/third_party/externals/angle2/ 806cda936..634909433 (1 commit)

Created with:
  roll-dep src/third_party/skia
BUG= 759402 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=stephana@chromium.org

Change-Id: Ifd949ace97ca08dcbec5675f429683408dcb86c1
Reviewed-on: https://chromium-review.googlesource.com/670070
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502497}
[modify] https://crrev.com/f49754fb9b9d92639cddea6638b344c507bc5aed/DEPS

Project Member

Comment 28 by bugdroid1@chromium.org, Sep 19 2017

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

commit f4a85e0ddd183a7bf7cdd0290875419085556fed
Author: Geoff Lang <geofflang@chromium.org>
Date: Tue Sep 19 19:40:55 2017

Roll ANGLE 371ed53..abd3135

https://chromium.googlesource.com/angle/angle.git/+log/371ed53..abd3135

BUG=,None,chromium:507755,697758,chromium:759402,chromium:765363

TBR=jmadill@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Change-Id: Id871246203c86489e25aac00149b1590cdb5f3e0
Reviewed-on: https://chromium-review.googlesource.com/673403
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502921}
[modify] https://crrev.com/f4a85e0ddd183a7bf7cdd0290875419085556fed/DEPS

The latest update on this compiler bug is:

"This has been fixed in the Visual Studio 15.4 Preview 3 release. You can download it here: https://www.visualstudio.com/en-us/news/releasenotes/vs2017-preview-relnotes"

That is from: https://developercommunity.visualstudio.com/content/problem/104890/code-gen-bug-in-1532-x64-compiler.html and https://developercommunity.visualstudio.com/content/problem/99372/vs-2017-1531-vectorization-issue.html

When I get a chance I will try the preview version to verify the fix.

Status: Fixed (was: Assigned)
I've confirmed that VS 2017 15.4.0 Preview 4.0 (_MSC_FULL_VER = 191125547) fixes this bug. I disabled the #pragma optimize hack in es2fShaderMatrixTests.cpp, reproduced the bug, then built with the latest preview build and confirmed that the bug no longer happens.

Because the #pragma optimize hacks in es2fShaderMatrixTests.cpp and es3fShaderMatrixTests.cpp are tagged to a particular compiler version they will automatically go away when a new compiler is used. Therefore there is no hurry to remove them, especially if some users may build these tests with Update 3.2 after Chromium has moved on.

Therefore there is not anything left to do here so I am closing this as fixed.

Note that next time I add compiler bug workarounds I should explicitly exclude clang to avoid the risk that the hacks will inadvertently apply to clang-cl. This should not happen in this case because clang doesn't match _MSC_FULL_VER version numbers exactly.

Status: Verified (was: Fixed)
I've confirmed that VS 2017 update 4 (version 15.4) fixes this bug. I did this by removing the workarounds, reproducing the bug with 15.3.2, then rebuilding with 15.4.

Note that the workarounds should be left for the foreseeable future. They are tightly scoped to 15.3.2 only so they are harmless, and 15.3.2 is still the default compiler for Chromium.
Project Member

Comment 32 by bugdroid1@chromium.org, Oct 19 2017

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

commit 70155edf7d3f4ffa931adc3077968ee99b9462ac
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu Oct 19 21:00:20 2017

VS 2017 15.4 with 10.0.15063.468 SDK

This change switches the VS 2017 package to use VS 2017 Update 4 while
still using the 10.0.15063.468 SDK. Update 4 fixes at least one code-gen
bug ( crbug.com/759402 ) but the 10.0.16299.0 SDK is still incompatible
with Chrome ( crbug.com/773476 ).

Packaging was done on a Windows Server 2016 VM, cleanly created for this
purpose.

Compiler was packaged up by downloading VS 2017 Update 4, from
https://www.visualstudio.com/vs/, and then passing these parameters to
the installer:

    --add Microsoft.VisualStudio.Workload.NativeDesktop
    --add Microsoft.VisualStudio.Component.VC.ATLMFC --includeRecommended
    --passive

Then the Windows 10.0.15063.468 SDK installer was run and everything was
installed except for the Windows Performance Toolkit, .Net Framework,
and the arm SDKs.

Then the packaging script was run like this:

    python depot_tools\win_toolchain\package_from_installed.py 2017 -w 10.0.15063.0

Bug:  773476 , 759402 
Change-Id: Ie2176b5ff765d9e5497f51a7b00c02fad04fb973
Reviewed-on: https://chromium-review.googlesource.com/727523
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510207}
[modify] https://crrev.com/70155edf7d3f4ffa931adc3077968ee99b9462ac/build/vs_toolchain.py

Project Member

Comment 33 by bugdroid1@chromium.org, Nov 4 2017

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

commit db45606398cf4389bf332b0cdcffd04e7de4a4f6
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri Nov 03 23:48:27 2017

Revert "VS 2017 15.4 with 10.0.15063.468 SDK"

This reverts commit 70155edf7d3f4ffa931adc3077968ee99b9462ac.

Reason for revert: the 15.4 toolchain breaks incremental linking for
Chrome ( crbug.com/780161 )

Original change's description:
> VS 2017 15.4 with 10.0.15063.468 SDK
> 
> This change switches the VS 2017 package to use VS 2017 Update 4 while
> still using the 10.0.15063.468 SDK. Update 4 fixes at least one code-gen
> bug ( crbug.com/759402 ) but the 10.0.16299.0 SDK is still incompatible
> with Chrome ( crbug.com/773476 ).
> 
> Packaging was done on a Windows Server 2016 VM, cleanly created for this
> purpose.
> 
> Compiler was packaged up by downloading VS 2017 Update 4, from
> https://www.visualstudio.com/vs/, and then passing these parameters to
> the installer:
> 
>     --add Microsoft.VisualStudio.Workload.NativeDesktop
>     --add Microsoft.VisualStudio.Component.VC.ATLMFC --includeRecommended
>     --passive
> 
> Then the Windows 10.0.15063.468 SDK installer was run and everything was
> installed except for the Windows Performance Toolkit, .Net Framework,
> and the arm SDKs.
> 
> Then the packaging script was run like this:
> 
>     python depot_tools\win_toolchain\package_from_installed.py 2017 -w 10.0.15063.0
> 
> Bug:  773476 , 759402 
> Change-Id: Ie2176b5ff765d9e5497f51a7b00c02fad04fb973
> Reviewed-on: https://chromium-review.googlesource.com/727523
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510207}

TBR=thakis@chromium.org,brucedawson@chromium.org,scottmg@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  773476 ,  759402 
Change-Id: Ica63e9740c954499b85ee891fb3bec0d48dd70fb
Reviewed-on: https://chromium-review.googlesource.com/753422
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513965}
[modify] https://crrev.com/db45606398cf4389bf332b0cdcffd04e7de4a4f6/build/vs_toolchain.py

Sign in to add a comment