New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
thakis@chromium.org
Closed: Jun 2016
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 595118
Blocking:
issue 82385



Sign in to add a comment
link

Issue 592745: Stop passing /FIIntrin.h to clang-cl

Reported by thakis@chromium.org, Mar 7 2016 Project Member 
Currently we force-include Intrin.h into all translation units with clang. There isn't a great reason for this.


Things that break when we stop doing it:


A file in libyuv uses _xgetbv but explicitly doesn't include the right header if defined(__clang__):

[1301/28561] CXX obj\third_party\libyuv\source\libyuv.cpu_id.obj
FAILED: ninja -t msvc -e environment.x86 -- "..\..\third_party/llvm-build/Release+Asserts/bin/clang-cl" -m32 /nologo /sh
owIncludes /FC @obj\third_party\libyuv\source\libyuv.cpu_id.obj.rsp /c ..\..\third_party\libyuv\source\cpu_id.cc /Foobj\
third_party\libyuv\source\libyuv.cpu_id.obj /Fdobj\third_party\libyuv\libyuv.cc.pdb
..\..\third_party\libyuv\source\cpu_id.cc(121,19) :  error: use of undeclared identifier '_xgetbv'
  xcr0 = (uint32)(_xgetbv(0));  // VS2010 SP1 required.
                  ^
1 error generated.



Several .c files in NaCl rely on functions not requiring a declaration in pre-c99 c, these just need explicit includes of intrin.h:

[3665/28551] CC obj\native_client\src\trusted\service_runtime\arch\x86_32\service_runtime_x86_32.sel_rt_32.obj
FAILED: ninja -t msvc -e environment.x86 -- "..\..\third_party/llvm-build/Release+Asserts/bin/clang-cl" -m32 /nologo /sh
owIncludes /FC @obj\native_client\src\trusted\service_runtime\arch\x86_32\service_runtime_x86_32.sel_rt_32.obj.rsp /c ..
\..\native_client\src\trusted\service_runtime\arch\x86_32\sel_rt_32.c /Foobj\native_client\src\trusted\service_runtime\a
rch\x86_32\service_runtime_x86_32.sel_rt_32.obj /Fdobj\native_client\src\trusted\service_runtime\arch\x86_32\service_run
time_x86_32.c.pdb
..\..\native_client\src\trusted\service_runtime\arch\x86_32\sel_rt_32.c(110,23) :  error: implicit declaration of functi
on '_mm_getcsr' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    ntcp->sys_mxcsr = _mm_getcsr();
                      ^
1 error generated.
[3728/28551] CC obj\native_client\src\shared\platform\win\platform64.nacl_exit.obj
FAILED: ninja -t msvc -e environment.x64 -- "..\..\third_party/llvm-build/Release+Asserts/bin/clang-cl" -m64 /nologo /sh
owIncludes /FC @obj\native_client\src\shared\platform\win\platform64.nacl_exit.obj.rsp /c ..\..\native_client\src\shared
\platform\win\nacl_exit.c /Foobj\native_client\src\shared\platform\win\platform64.nacl_exit.obj /Fdobj\native_client\src
\shared\platform\platform64.c.pdb
..\..\native_client\src\shared\platform\win\nacl_exit.c(41,5) :  error: implicit declaration of function '__halt' is inv
alid in C99 [-Werror,-Wimplicit-function-declaration]
    __halt();
    ^
1 error generated.
[3740/28551] CC obj\native_client\src\shared\platform\win\platform64.nacl_time.obj
FAILED: ninja -t msvc -e environment.x64 -- "..\..\third_party/llvm-build/Release+Asserts/bin/clang-cl" -m64 /nologo /sh
owIncludes /FC @obj\native_client\src\shared\platform\win\platform64.nacl_time.obj.rsp /c ..\..\native_client\src\shared
\platform\win\nacl_time.c /Foobj\native_client\src\shared\platform\win\platform64.nacl_time.obj /Fdobj\native_client\src
\shared\platform\platform64.c.pdb
..\..\native_client\src\shared\platform\win\nacl_time.c(96,3) :  error: implicit declaration of function '__cpuid' is in
valid in C99 [-Werror,-Wimplicit-function-declaration]
  __cpuid(cpu_info, kBaseInfoPage);
  ^
..\..\native_client\src\shared\platform\win\nacl_time.c(96,3) :  note: did you mean '__cpuidex'?
c:\src\depot_tools\win_toolchain\vs2013_files\4087e065abebdca6dbd0caca2910c6718d2ec67f\win_sdk\Include\10.0.10586.0\um\w
innt.h(3020,1) :  note: '__cpuidex' declared here
__cpuidex (
^
1 error generated.
[3906/28551] CC obj\native_client\src\shared\platform\win\platform.nacl_exit.obj
FAILED: ninja -t msvc -e environment.x86 -- "..\..\third_party/llvm-build/Release+Asserts/bin/clang-cl" -m32 /nologo /sh
owIncludes /FC @obj\native_client\src\shared\platform\win\platform.nacl_exit.obj.rsp /c ..\..\native_client\src\shared\p
latform\win\nacl_exit.c /Foobj\native_client\src\shared\platform\win\platform.nacl_exit.obj /Fdobj\native_client\src\sha
red\platform\platform.c.pdb
..\..\native_client\src\shared\platform\win\nacl_exit.c(41,5) :  error: implicit declaration of function '__halt' is inv
alid in C99 [-Werror,-Wimplicit-function-declaration]
    __halt();
    ^
1 error generated.
[3908/28551] CC obj\native_client\src\shared\platform\win\platform.nacl_time.obj
FAILED: ninja -t msvc -e environment.x86 -- "..\..\third_party/llvm-build/Release+Asserts/bin/clang-cl" -m32 /nologo /sh
owIncludes /FC @obj\native_client\src\shared\platform\win\platform.nacl_time.obj.rsp /c ..\..\native_client\src\shared\p
latform\win\nacl_time.c /Foobj\native_client\src\shared\platform\win\platform.nacl_time.obj /Fdobj\native_client\src\sha
red\platform\platform.c.pdb
..\..\native_client\src\shared\platform\win\nacl_time.c(96,3) :  error: implicit declaration of function '__cpuid' is in
valid in C99 [-Werror,-Wimplicit-function-declaration]
  __cpuid(cpu_info, kBaseInfoPage);
  ^
..\..\native_client\src\shared\platform\win\nacl_time.c(96,3) :  note: did you mean '__cpuidex'?
c:\src\depot_tools\win_toolchain\vs2013_files\4087e065abebdca6dbd0caca2910c6718d2ec67f\win_sdk/Include/10.0.10586.0/um\w
innt.h(6938,1) :  note: '__cpuidex' declared here
__cpuidex (
^
1 error generated.
[4021/28546] CC obj\native_client\src\trusted\service_runtime\sel.sel_ldr.obj
FAILED: ninja -t msvc -e environment.x86 -- "..\..\third_party/llvm-build/Release+Asserts/bin/clang-cl" -m32 /nologo /sh
owIncludes /FC @obj\native_client\src\trusted\service_runtime\sel.sel_ldr.obj.rsp /c ..\..\native_client\src\trusted\ser
vice_runtime\sel_ldr.c /Foobj\native_client\src\trusted\service_runtime\sel.sel_ldr.obj /Fdobj\native_client\src\trusted
\service_runtime\sel.c.pdb
..\..\native_client\src\trusted\service_runtime\sel_ldr.c(972,3) :  error: implicit declaration of function '_ReadWriteB
arrier' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  _ReadWriteBarrier();
  ^



And then there's this error in libopenjpeg that I don't understand yet; probably related to .c files somehow too:

[138/28561] CC obj\third_party\pdfium\third_party\libopenjpeg20\fx_libopenjpeg.dwt.obj
FAILED: ninja -t msvc -e environment.x86 -- "..\..\third_party/llvm-build/Release+Asserts/bin/clang-cl" -m32 /nologo /sh
owIncludes /FC @obj\third_party\pdfium\third_party\libopenjpeg20\fx_libopenjpeg.dwt.obj.rsp /c ..\..\third_party\pdfium\
third_party\libopenjpeg20\dwt.c /Foobj\third_party\pdfium\third_party\libopenjpeg20\fx_libopenjpeg.dwt.obj /Fdobj\third_
party\pdfium\third_party\fx_libopenjpeg.c.pdb
In file included from ..\..\third_party\pdfium\third_party\libopenjpeg20\dwt.c:44:
In file included from ..\..\third_party\pdfium\third_party\libopenjpeg20/opj_includes.h:125:
In file included from C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\intrin.h:3
4:
In file included from C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\x86intrin.
h:29:
In file included from C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\immintrin.
h:37:
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\smmintrin.h(303,67) :  error: in
valid conversion between vector type '__m128i' (vector of 2 'long long' values) and integer type '__v16qs' (aka 'signed
char') of different size
  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qs)__V, (__v16qs)__V, 0, 1), __v2di);
                                                                  ^~~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\smmintrin.h(303,81) :  error: in
valid conversion between vector type '__m128i' (vector of 2 'long long' values) and integer type '__v16qs' (aka 'signed
char') of different size
  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qs)__V, (__v16qs)__V, 0, 1), __v2di);
                                                                                ^~~~~~~~~~~~
In file included from ..\..\third_party\pdfium\third_party\libopenjpeg20\dwt.c:44:
In file included from ..\..\third_party\pdfium\third_party\libopenjpeg20/opj_includes.h:125:
In file included from C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\intrin.h:3
4:
In file included from C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\x86intrin.
h:29:
In file included from C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\immintrin.
h:41:
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(77,46) :  error: pas
sing '__v4df' (aka 'double') to parameter of incompatible type '__attribute__((__vector_size__(4 * sizeof(double)))) dou
ble' (vector of 4 'double' values)
  return (__m256d)__builtin_ia32_addsubpd256((__v4df)__a, (__v4df)__b);
                                             ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(83,45) :  error: pas
sing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) float
' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_addsubps256((__v8sf)__a, (__v8sf)__b);
                                            ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(101,43) :  error: pa
ssing '__v4df' (aka 'double') to parameter of incompatible type '__attribute__((__vector_size__(4 * sizeof(double)))) do
uble' (vector of 4 'double' values)
  return (__m256d)__builtin_ia32_maxpd256((__v4df)__a, (__v4df)__b);
                                          ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(107,42) :  error: pa
ssing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) floa
t' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_maxps256((__v8sf)__a, (__v8sf)__b);
                                         ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(113,43) :  error: pa
ssing '__v4df' (aka 'double') to parameter of incompatible type '__attribute__((__vector_size__(4 * sizeof(double)))) do
uble' (vector of 4 'double' values)
  return (__m256d)__builtin_ia32_minpd256((__v4df)__a, (__v4df)__b);
                                          ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(119,42) :  error: pa
ssing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) floa
t' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_minps256((__v8sf)__a, (__v8sf)__b);
                                         ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(137,44) :  error: pa
ssing '__v4df' (aka 'double') to parameter of incompatible type '__attribute__((__vector_size__(4 * sizeof(double)))) do
uble' (vector of 4 'double' values)
  return (__m256d)__builtin_ia32_sqrtpd256((__v4df)__a);
                                           ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(143,43) :  error: pa
ssing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) floa
t' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_sqrtps256((__v8sf)__a);
                                          ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(149,44) :  error: pa
ssing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) floa
t' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_rsqrtps256((__v8sf)__a);
                                           ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(155,42) :  error: pa
ssing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) floa
t' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_rcpps256((__v8sf)__a);
                                         ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(222,44) :  error: pa
ssing '__v4df' (aka 'double') to parameter of incompatible type '__attribute__((__vector_size__(4 * sizeof(double)))) do
uble' (vector of 4 'double' values)
  return (__m256d)__builtin_ia32_haddpd256((__v4df)__a, (__v4df)__b);
                                           ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(228,43) :  error: pa
ssing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) floa
t' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_haddps256((__v8sf)__a, (__v8sf)__b);
                                          ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(234,44) :  error: pa
ssing '__v4df' (aka 'double') to parameter of incompatible type '__attribute__((__vector_size__(4 * sizeof(double)))) do
uble' (vector of 4 'double' values)
  return (__m256d)__builtin_ia32_hsubpd256((__v4df)__a, (__v4df)__b);
                                           ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(240,43) :  error: pa
ssing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) floa
t' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_hsubps256((__v8sf)__a, (__v8sf)__b);
                                          ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(253,50) :  error: pa
ssing '__v4df' (aka 'double') to parameter of incompatible type '__attribute__((__vector_size__(4 * sizeof(double)))) do
uble' (vector of 4 'double' values)
  return (__m256d)__builtin_ia32_vpermilvarpd256((__v4df)__a, (__v4di)__c);
                                                 ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(265,49) :  error: pa
ssing '__v8sf' (aka 'float') to parameter of incompatible type '__attribute__((__vector_size__(8 * sizeof(float)))) floa
t' (vector of 8 'float' values)
  return (__m256)__builtin_ia32_vpermilvarps256((__v8sf)__a, (__v8si)__c);
                                                ^~~~~~~~~~~
C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.9.0\include\avxintrin.h(333,5) :  error: pas
sing '__v4df' (aka 'double') to parameter of incompatible type '__attribute__((__vector_size__(4 * sizeof(double)))) dou
ble' (vector of 4 'double' values)
    (__v4df)__a, (__v4df)__b, (__v4df)__c);
    ^~~~~~~~~~~

Comment 1 by thakis@chromium.org, Mar 7 2016 

Maps functions to required header: https://msdn.microsoft.com/en-us/library/hh977022.aspx

Comment 2 by thakis@chromium.org, Mar 8 2016 

winnt.h apparently declares _mm_getcsr and also has a `#pragma intrinsic` line for that.

Comment 3 by thakis@chromium.org, Mar 8 2016 

It also has the same for __stosb and its inline RtlSecureZeroMemory() calls that __stosb.

It kind of feels like doing this for real is somewhat blocked on enough pragma intrinsic support in clang-cl that it can handle the ones defined in system headers.

Comment 4 by thakis@chromium.org, Mar 8 2016 

Fix for some of the nacl stuff: https://codereview.chromium.org/1778653002/

Comment 5 by thakis@chromium.org, Mar 8 2016 

Fix for the libyuv issue: https://codereview.chromium.org/1780473003/

Comment 7 by bugdroid1@chromium.org, Mar 10 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/libyuv/libyuv.git/+/76aee8ced7ca74c724d69c1dcf9891348450c8e8

commit 76aee8ced7ca74c724d69c1dcf9891348450c8e8
Author: Frank Barchard <fbarchard@google.com>
Date: Thu Mar 10 22:01:26 2016

Remove most clang-cl special cases from cpu_id.cc

They are not needed, and due to them there was a call to _xgetbv()
without a declaration of the function.  This used to work because we
implicitly included intrin.h in all translation units with clang-cl, but
we want to stop doing that.

BUG= chromium:592745 
R=fbarchard@google.com

Review URL: https://codereview.chromium.org/1780473003 .

[modify] https://crrev.com/76aee8ced7ca74c724d69c1dcf9891348450c8e8/source/cpu_id.cc

Comment 9 by thakis@chromium.org, Mar 15 2016 

Note to self:

LLVM pragma intrin bug: https://llvm.org/bugs/show_bug.cgi?id=19898
 Bug 505314  also talks about pragma intrin a bit.

Comment 10 by thakis@chromium.org, Mar 15 2016 

For fx_libopenjpeg: opj_includes.h contains this snippet before including intrin.h which confuses clang a lot:

/* Ignore GCC attributes if this is not GCC */
#ifndef __GNUC__
	#define __attribute__(x) /* __attribute__(x) */
#endif



__GNUC__ isn't defined in clang-cl builds, so that gets executed. Redefining __attribute__ seems super unfriendly; openjpeg should use some ATTRIB macro instead...

Comment 11 by thakis@chromium.org, Mar 17 2016 

Current status: Almost everything builds. With the following two changes, everything builds without /FIIntrin.h (in the one build config I tried locally, at least) (and if I locally hack around the openjpeg thing in comment 10; this still needs a real fix too):

C:\src\chrome\src\native_client>git diff
diff --git a/src/shared/platform/win/nacl_secure_random.c b/src/shared/platform/win/nacl_secure_random.c
index f549441..342b065 100644
--- a/src/shared/platform/win/nacl_secure_random.c
+++ b/src/shared/platform/win/nacl_secure_random.c
@@ -9,6 +9,9 @@
  */
 #include <windows.h>

+/* TODO(thakis): remove, for __stosb used implicitly by SecureZeroMemory */
+#include <intrin.h>
+
 /*
  * #define needed to link in RtlGenRandom(), a.k.a. SystemFunction036. See
  * the "Community Additions" comment on MSDN here:
diff --git a/src/trusted/service_runtime/arch/x86_64/sel_rt_64.c b/src/trusted/service_runtime/arch/x86_64/sel_rt_64.c
index 63769e3..6c96a0c 100644
--- a/src/trusted/service_runtime/arch/x86_64/sel_rt_64.c
+++ b/src/trusted/service_runtime/arch/x86_64/sel_rt_64.c
@@ -18,6 +18,9 @@
 #include <xmmintrin.h>
 #endif

+/* TODO(thakis): remove, for _mm_getcsr, not sure why this compiles? */
+#include <intrin.h>
+
 #include "native_client/src/shared/platform/nacl_log.h"
 #include "native_client/src/trusted/service_runtime/nacl_app_thread.h"
 #include "native_client/src/trusted/service_runtime/nacl_signal.h"





The first one is needed due to the explanation in comment 10 in the LLVM bug linked to in comment 9.


The second one is needed 'cause the file calls _mm_getcsr but includes mmintrin.h instead of intrin.h for it incorrectly. So I understand the fix, but I don't understand why this isn't a compiler diag ("error: implicit declaration of function" or something, like above). Maybe sel_rt_64.c isn't built in C99 mode?

Comment 12 by thakis@chromium.org, Mar 17 2016 

Re last point: That's because winnt.h declares (but doesn't define) _mm_getcsr(). So just fixing the include will help there.

Comment 13 by thakis@chromium.org, Mar 17 2016 

Filed https://github.com/uclouvain/openjpeg/issues/727 for the openjpeg thing.

Comment 14 by bugdroid1@chromium.org, Mar 17 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/native_client/src/native_client.git/+/06cacd6becc3f033d03fab97e6fa9ef99d9b41f2

commit 06cacd6becc3f033d03fab97e6fa9ef99d9b41f2
Author: thakis <thakis@chromium.org>
Date: Thu Mar 17 23:23:23 2016

win: Fix another intrinsic include.

_mm_getcsr is declared in intrin.h, see
https://msdn.microsoft.com/en-us/library/hh977022.aspx
This worked because transitive includes ended up pulling in winnt.h,
which also declares this function (and tells the compiler to use a
built-in).  The built-in mechanism doesn't work with clang-cl
(see http://llvm.org/PR19898 comment 10) and leads to a linker error.
Since the file tries to include the right header for _mm_getcsr, just
fix the include, which happens to make things link with clang-cl too.

This should have been part of https://codereview.chromium.org/1778653002/

BUG= https://crbug.com/592745 

Review URL: https://codereview.chromium.org/1808123003

[modify] https://crrev.com/06cacd6becc3f033d03fab97e6fa9ef99d9b41f2/src/trusted/service_runtime/arch/x86_64/sel_rt_64.c

Comment 15 by bugdroid1@chromium.org, Mar 18 2016

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

commit b6cbb648f14d80e86bda9b65e42dbbf016cea4fc
Author: thakis <thakis@chromium.org>
Date: Fri Mar 18 20:19:42 2016

Roll PDFium 30410ce..a051db3

https://pdfium.googlesource.com/pdfium.git/+log/30410ce..a051db3

BUG= 592745 , 589724 

TEST=bots
TBR=tsepez

Review URL: https://codereview.chromium.org/1817513003

Cr-Commit-Position: refs/heads/master@{#382066}

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

Comment 16 by thakis@chromium.org, Mar 18 2016 

Status: All but the __stosb linker error due to SecureZeroMemory() being an inline function but winnt.h not including intrin.h are resolved.

Comment 17 by bugdroid1@chromium.org, Jun 16 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/native_client/src/native_client.git/+/8d49f6b628879a41bac93660f27bf694e6bc7143

commit 8d49f6b628879a41bac93660f27bf694e6bc7143
Author: thakis <thakis@chromium.org>
Date: Thu Jun 16 01:45:30 2016

win: Add an explicit intrin.h include to work around a clang-cl bug.

It's admittedly pretty lame to put this workaround in, but last I checked this
was the only file needing this workaround, and force-including intrin.h in
_every_ translation unit isn't great either.  So let's add this workaround for
now so we can remove that force-inclusion.

BUG= chromium:592745 

Review-Url: https://codereview.chromium.org/2070533002

[modify] https://crrev.com/8d49f6b628879a41bac93660f27bf694e6bc7143/src/shared/platform/win/nacl_secure_random.c

Comment 18 by bugdroid1@chromium.org, Jun 16 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/917527bafff5217c95f343fd8a4926dc8c813507

commit 917527bafff5217c95f343fd8a4926dc8c813507
Author: nacl-deps-roller <nacl-deps-roller@chromium.org>
Date: Thu Jun 16 04:42:52 2016

Roll src/native_client/ 33ea6a023..8d49f6b62 (1 commit).

https://chromium.googlesource.com/native_client/src/native_client.git/+log/33ea6a02332d..8d49f6b62887

$ git log 33ea6a023..8d49f6b62 --date=short --no-merges --format='%ad %ae %s'
2016-06-15 thakis win: Add an explicit intrin.h include to work around a clang-cl bug.

BUG= 592745 

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_dbg_32_ng,linux_nacl_sdk_build
TBR=mseaborn@chromium.org

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

[modify] https://crrev.com/917527bafff5217c95f343fd8a4926dc8c813507/DEPS

Comment 19 by bugdroid1@chromium.org, Jun 16 2016

Project Member 
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/2b360714abf9395d85ac9b50850fcd2fc4bda442

commit 2b360714abf9395d85ac9b50850fcd2fc4bda442
Author: Nico Weber <thakis@chromium.org>
Date: Thu Jun 16 08:12:05 2016

win: Add an explicit intrin.h include to work around a clang-cl bug.

I did the same change in NaCl in
https://codereview.chromium.org/2070533002/.  I thought NaCl is the only
place where this was needed, but at least it's due to SecureZeroMemory()
again.  So it's two files now, but at least there's only one function we
know of that needs this, and it's only called in three files total in
all projects used by Chromium.

BUG= chromium:592745 

Change-Id: I07ed197869e26ec70c1f4b75d91fd64abae5015e
Reviewed-on: https://boringssl-review.googlesource.com/8320
Reviewed-by: David Benjamin <davidben@google.com>

[modify] https://crrev.com/2b360714abf9395d85ac9b50850fcd2fc4bda442/crypto/mem.c

Comment 20 by bugdroid1@chromium.org, Jun 22 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/webm/libvpx/+/8c29d332c9a0d9db19b916d4910935d5ee6b6338

commit 8c29d332c9a0d9db19b916d4910935d5ee6b6338
Author: Nico Weber <thakis@chromium.org>
Date: Wed Jun 22 18:44:47 2016

win: Include <intrin.h> instead of manually declaring intrinsics.

This helps clang-cl.

BUG= chromium:592745 

Change-Id: I49f9b6928c91b2b43567f4336520ba4f1010d3ad

[modify] https://crrev.com/8c29d332c9a0d9db19b916d4910935d5ee6b6338/vpx_ports/x86.h

Comment 22 by bugdroid1@chromium.org, Jun 22 2016

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

commit c03d07eb1deef9057e14ad26ee26adb1f7be2362
Author: thakis <thakis@chromium.org>
Date: Wed Jun 22 21:27:20 2016

Roll src/third_party/libvpx/source/libvpx/ 181988d37..243029faf (45 commits).

https://chromium.googlesource.com/webm/libvpx.git/+log/181988d37273..243029faff45

$ git log 181988d37..243029faf --date=short --no-merges --format='%ad %ae %s'
2016-06-22 thakis win: Include <intrin.h> instead of manually declaring intrinsics.
2016-06-22 yaowu Prevent negative variance
2016-06-20 yaowu Fix ubsan warnings: vp9/encoder/vp9_mcomp.c
2016-06-15 huisu Add a hardware compatibility feature
2016-06-20 yaowu Make type conversion explicit
2016-06-19 johannkoenig Fix iosbuild.sh: outdated reference to ios-version.sh
2016-06-18 jzern realtime_test: remove decoded frame count check
2016-06-17 jingning Refactor optimize_b for speed performance
2016-06-17 jingning Port optimize_b speed-up from vp10
2016-06-17 yaowu Fix ubsan warning: vp9/encoder/vp9_encodeframe.c
2016-06-17 jingning Use 64-bit integer to store distortion in optimize_b
2016-06-17 johannkoenig Configuration: Use 'is_in' instead of echo | grep
2016-06-10 johannkoenig Parse codec options in order of occurrence.
2016-06-15 tomfinegan Disable neon for Xcode versions < v6.3.
2016-06-17 gezalore Fix warnings from gtest under GCC 5 or newer.
2016-06-15 tomfinegan Output frames in first pass for VPX_DL_REALTIME.
2016-06-17 yaowu Fix ubsan warnings: vp9/encoder/vp9_encoder.c
2016-06-16 johannkoenig Remove ios-version.sh
2016-06-13 johannkoenig vp8_[cd]x_iface.c: Initialize structures to 0
2016-06-13 johannkoenig VP8_COMP: make frames_since_golden signed
(...)

R=johannkoenig@chromium.org
BUG= 592745 

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

[modify] https://crrev.com/c03d07eb1deef9057e14ad26ee26adb1f7be2362/DEPS
[modify] https://crrev.com/c03d07eb1deef9057e14ad26ee26adb1f7be2362/third_party/libvpx/README.chromium
[modify] https://crrev.com/c03d07eb1deef9057e14ad26ee26adb1f7be2362/third_party/libvpx/source/config/linux/arm-neon-cpu-detect/vpx_dsp_rtcd.h
[modify] https://crrev.com/c03d07eb1deef9057e14ad26ee26adb1f7be2362/third_party/libvpx/source/config/linux/arm-neon/vpx_dsp_rtcd.h
[modify] https://crrev.com/c03d07eb1deef9057e14ad26ee26adb1f7be2362/third_party/libvpx/source/config/linux/arm64/vpx_dsp_rtcd.h
[modify] https://crrev.com/c03d07eb1deef9057e14ad26ee26adb1f7be2362/third_party/libvpx/source/config/vpx_version.h

Comment 23 by bugdroid1@chromium.org, Jun 23 2016

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

commit ac7670452f51ecfd1061749b3d5e15f8544a55ee
Author: thakis <thakis@chromium.org>
Date: Thu Jun 23 15:35:42 2016

clang/win: Stop force-including intrin.h everywhere.

MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics.
When an intrinsic is called, the compiler can emit specialized code for intrinsics.
(It's free to emit a regular call to a function too though.)

intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports
`#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something
that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call.  With this,
it is possible to manually declare an intrinsic and then use that pragma, and since it'll be
treated as an intrinsic, no linker error will happen.

clang-cl doesn't yet implement `#pragma intrinsic`, so e.g.

  void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue);
  #pragma intrinsic(__cpuidex)
  // later, call __cpuidex()

will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then
clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function
that isn't defined anywhere.

This is not a problem: Just #include <intrin.h> instead of manually declaring the function.
With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and
no linker error will be emitted (and the definition is always_inline, so it's fast).

There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare
intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions
defined in system headers.  So if some of our code calls one of these other inline functions,
it needs intrin.h without us knowing about it.  Luckily, we know only a single function where
this is an issue in practice: SecureZeroMemory(), which calls __stosb.  So we had to add
explicit and mysterious includes for <intrin.h> to the two files that call that function. If
more examples of this pop up, we can reevaluate if we want to force-include this header
everywhere, but for now it seems overkill to inject this header into every translation unit
just because two translation units need it.

BUG= 592745 

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

[modify] https://crrev.com/ac7670452f51ecfd1061749b3d5e15f8544a55ee/build/common.gypi
[modify] https://crrev.com/ac7670452f51ecfd1061749b3d5e15f8544a55ee/build/config/win/BUILD.gn

Comment 24 by thakis@chromium.org, Jun 23 2016

Owner: thakis@chromium.org
Status: Fixed (was: Untriaged)

Comment 25 by bugdroid1@chromium.org, Jul 14 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/053185feb2645e1628e6b25fbf1e21f309c26776

commit 053185feb2645e1628e6b25fbf1e21f309c26776
Author: fbarchard <fbarchard@google.com>
Date: Thu Jul 14 17:32:28 2016

roll libyuv from r1579 to r1607

roll libyuv for GN fix to chromium internal settings, enabling more warnings
and consistant with gyp build options.

Add ARGBExtractAlpha function.
Fix YUV bilinear filter improved filter accuracy; uses 128-f instead of 127-f.
unittest fixes for Arm scaler - use J420 for J422 pointing to same row,
improving performance and accuracy.
Fix ios jpeg build error.
Fix BlendPlaneRow_SSSE3 impossible constraints on android full debug
Fix unittests to avoid private row.h header.
Fix vector_size(32) build error on yuv conversion for intel on gcc 4.4.

BUG= libyuv:572 , chromium:592745 , libyuv:575 , libyuv:579 , libyuv:573 , libyuv:583 , chromium:601140 , libyuv:582 , libyuv:581 , chromium:601140 , libyuv:578 , libyuv:581 , libyuv:580 , libyuv:586 , libyuv:590 , webrtc:5006 , libyuv:593 , libyuv:592 , libyuv:594 , libyuv:595 , libyuv:597 , libyuv:599 , libyuv:598 , libyuv:600 , libyuv:602 , libyuv:605 , libyuv:610 , libyuv:612 ,  libyuv:618 ,  libyuv:620 ,  webrtc:6091 ,  webrtc:6094 ,  libyuv:616 ,  libyuv:623 
TEST=try bots
R=rsesek@chromium.org

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

[modify] https://crrev.com/053185feb2645e1628e6b25fbf1e21f309c26776/DEPS

Comment 26 by bugdroid1@chromium.org, Jul 14 2016

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

commit d35937737cf9706dd535b72b7a13f94993d32187
Author: wjmaclean <wjmaclean@chromium.org>
Date: Thu Jul 14 18:44:12 2016

Revert of roll libyuv from r1579 to r1607 (patchset #19 id:350001 of https://codereview.chromium.org/2126333002/ )

Reason for revert:
Causing compile failures on:

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20MSAN

Original issue's description:
> roll libyuv from r1579 to r1607
>
> roll libyuv for GN fix to chromium internal settings, enabling more warnings
> and consistant with gyp build options.
>
> Add ARGBExtractAlpha function.
> Fix YUV bilinear filter improved filter accuracy; uses 128-f instead of 127-f.
> unittest fixes for Arm scaler - use J420 for J422 pointing to same row,
> improving performance and accuracy.
> Fix ios jpeg build error.
> Fix BlendPlaneRow_SSSE3 impossible constraints on android full debug
> Fix unittests to avoid private row.h header.
> Fix vector_size(32) build error on yuv conversion for intel on gcc 4.4.
>
> BUG= libyuv:572 , chromium:592745 , libyuv:575 , libyuv:579 , libyuv:573 , libyuv:583 , chromium:601140 , libyuv:582 , libyuv:581 , chromium:601140 , libyuv:578 , libyuv:581 , libyuv:580 , libyuv:586 , libyuv:590 , webrtc:5006 , libyuv:593 , libyuv:592 , libyuv:594 , libyuv:595 , libyuv:597 , libyuv:599 , libyuv:598 , libyuv:600 , libyuv:602 , libyuv:605 , libyuv:610 , libyuv:612 ,  libyuv:618 ,  libyuv:620 ,  webrtc:6091 ,  webrtc:6094 ,  libyuv:616 ,  libyuv:623 
> TEST=try bots
> R=rsesek@chromium.org
>
> Committed: https://crrev.com/053185feb2645e1628e6b25fbf1e21f309c26776
> Cr-Commit-Position: refs/heads/master@{#405512}

TBR=rsesek@chromium.org,fbarchard@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= libyuv:572 , chromium:592745 , libyuv:575 , libyuv:579 , libyuv:573 , libyuv:583 , chromium:601140 , libyuv:582 , libyuv:581 , chromium:601140 , libyuv:578 , libyuv:581 , libyuv:580 , libyuv:586 , libyuv:590 , webrtc:5006 , libyuv:593 , libyuv:592 , libyuv:594 , libyuv:595 , libyuv:597 , libyuv:599 , libyuv:598 , libyuv:600 , libyuv:602 , libyuv:605 , libyuv:610 , libyuv:612 ,  libyuv:618 ,  libyuv:620 ,  webrtc:6091 ,  webrtc:6094 ,  libyuv:616 ,  libyuv:623 

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

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

Comment 27 by bugdroid1@chromium.org, Oct 25 2016

Project Member 
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/351af19bb0f159bdc3417f7956a77cc9b682b719

commit 351af19bb0f159bdc3417f7956a77cc9b682b719
Author: Nico Weber <thakis@chromium.org>
Date: Tue Oct 25 20:49:33 2016

Remove a clang-cl workaround that's no longer needed.

clang-cl now supports enough of `#pragma intrinsic` that
it can use SecureZeroMemory() without an explicit intrin.h include.
This reverts https://boringssl-review.googlesource.com/#/c/8320/

BUG= chromium:592745 

Change-Id: Ib766133f1713137bddd07654376a3b4888d4b0fb
Reviewed-on: https://boringssl-review.googlesource.com/11780
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

[modify] https://crrev.com/351af19bb0f159bdc3417f7956a77cc9b682b719/crypto/mem.c

Comment 28 by bugdroid1@chromium.org, Oct 28 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/native_client/src/native_client.git/+/bba323f75e3d20d2d496a1986f91fc81b624affa

commit bba323f75e3d20d2d496a1986f91fc81b624affa
Author: thakis <thakis@chromium.org>
Date: Fri Oct 28 21:24:37 2016

win: Remove workaround for clang-cl that's no longer necessary.

clang-cl now understands enough of `#pragma intrinsic` that it's no longer
necessary to explicitly include intrin.h to use SecureZeroMemory().
Reverts https://codereview.chromium.org/2070533002

BUG= chromium:592745 

Review-Url: https://codereview.chromium.org/2453583002

[modify] https://crrev.com/bba323f75e3d20d2d496a1986f91fc81b624affa/src/shared/platform/win/nacl_secure_random.c

Comment 29 by bugdroid1@chromium.org, Oct 28 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/127359fc73b0010b3aa223aeae9dc7304ac624f0

commit 127359fc73b0010b3aa223aeae9dc7304ac624f0
Author: nacl-deps-roller <nacl-deps-roller@chromium.org>
Date: Fri Oct 28 22:35:44 2016

Roll src/native_client/ 631223764..bba323f75 (1 commit).

https://chromium.googlesource.com/native_client/src/native_client.git/+log/631223764fe3..bba323f75e3d

$ git log 631223764..bba323f75 --date=short --no-merges --format='%ad %ae %s'
2016-10-28 thakis win: Remove workaround for clang-cl that's no longer necessary.

BUG= 592745 

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.chromium.linux:linux_chromium_dbg_32_ng,linux_nacl_sdk_build
TBR=mseaborn@chromium.org

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

[modify] https://crrev.com/127359fc73b0010b3aa223aeae9dc7304ac624f0/DEPS
►

Sign in to add a comment