Avoid /arch:AVX[2] in Skia on Windows.

Avoid /arch:AVX[2] in Skia on Windows.

Same deal as the Skia CL (https://skia-review.googlesource.com/5005)
except that apparently here we use literals and not defines for each
SK_CPU_SSE_LEVEL.  AVX == 51, AVX2 == 52.

BUG=666707
COMMIT=false

[mtklein_C, 2016-11-18 18:15:24]

The CQ bit was checked by mtklein@chromium.org to run a CQ dry run

[mtklein_C, 2016-11-18 18:15:31]

mtklein@chromium.org changed reviewers:
+ brucedawson@chromium.org

[commit-bot: I haz the power, 2016-11-18 18:16:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brucedawson, 2016-11-18 18:21:30]

lgtm with line-length nit

https://codereview.chromium.org/2518473003/diff/20001/skia/BUILD.gn
File skia/BUILD.gn (right):

https://codereview.chromium.org/2518473003/diff/20001/skia/BUILD.gn#newcode197
skia/BUILD.gn:197: "/wd4752",  # We use AVX but don't let the compiler do so
itself.  See crbug.com/666707.
Wrap this line at 80 characters.

[mtklein_C, 2016-11-18 18:24:05]

The CQ bit was checked by mtklein@chromium.org

[mtklein_C, 2016-11-18 18:24:06]

https://codereview.chromium.org/2518473003/diff/20001/skia/BUILD.gn
File skia/BUILD.gn (right):

https://codereview.chromium.org/2518473003/diff/20001/skia/BUILD.gn#newcode197
skia/BUILD.gn:197: "/wd4752",  # We use AVX but don't let the compiler do so
itself.  See crbug.com/666707.
On 2016/11/18 at 18:21:30, brucedawson wrote:
> Wrap this line at 80 characters.

Whoops, as you may have guessed, my editor is set at 100.

Done.

[mtklein_C, 2016-11-18 18:24:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
brucedawson@chromium.org
Link to the patchset: https://codereview.chromium.org/2518473003/#ps40001
(title: "wrap")

[commit-bot: I haz the power, 2016-11-18 18:25:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mtklein_C, 2016-11-18 20:50:41]

PS 4 now passes -mavx, etc when is_clang on Windows (just like SSSE3 and SSE4
targets).

PTAL

[brucedawson, 2016-11-18 21:00:26]

I have some concerns. Also, I went a different route - more .gn code but less
variation from previous behavior:

https://codereview.chromium.org/2516703002/

https://codereview.chromium.org/2518473003/diff/60001/skia/BUILD.gn
File skia/BUILD.gn (right):

https://codereview.chromium.org/2518473003/diff/60001/skia/BUILD.gn#newcode557
skia/BUILD.gn:557: cflags = [ "-mavx" ]
It feels odd to use -mavx on a Windows build, but if it works...

https://codereview.chromium.org/2518473003/diff/60001/skia/BUILD.gn#newcode572
skia/BUILD.gn:572: if (!is_win || is_clang) {
This may have unintended consequences because now four previously unused flags
are being passed to clang-cl.

[mtklein_C, 2016-11-18 21:03:56]

https://codereview.chromium.org/2518473003/diff/60001/skia/BUILD.gn
File skia/BUILD.gn (right):

https://codereview.chromium.org/2518473003/diff/60001/skia/BUILD.gn#newcode557
skia/BUILD.gn:557: cflags = [ "-mavx" ]
On 2016/11/18 at 21:00:26, brucedawson wrote:
> It feels odd to use -mavx on a Windows build, but if it works...

Yeah.  It's the only way to express things like -mssse3 and -msse4.1, so we do
this for consistency.

https://codereview.chromium.org/2518473003/diff/60001/skia/BUILD.gn#newcode572
skia/BUILD.gn:572: if (!is_win || is_clang) {
On 2016/11/18 at 21:00:26, brucedawson wrote:
> This may have unintended consequences because now four previously unused flags
are being passed to clang-cl.

Hmm.  I think it should be okay, especially because it's restricted to only
clang-cl.  We've been using these flags with clang-not-cl for a while now.

[mtklein_C, 2016-11-18 22:05:01]

Description was changed from

==========
Avoid /arch:AVX[2] in Skia on Windows.

Same deal as the Skia CL (https://skia-review.googlesource.com/5005)
except that apparently here we use literals and not defines for each
SK_CPU_SSE_LEVEL.  AVX == 51, AVX2 == 52.

BUG=666707
==========

to

==========
Avoid /arch:AVX[2] in Skia on Windows.

Same deal as the Skia CL (https://skia-review.googlesource.com/5005)
except that apparently here we use literals and not defines for each
SK_CPU_SSE_LEVEL.  AVX == 51, AVX2 == 52.

BUG=666707
COMMIT=false
==========

[brucedawson, 2016-12-10 01:31:02]

Close this?