Break out SSE functions into new media_sse target.

Break out SSE functions into new media_sse target.

It turns out Chrome doesn't have a minimum requirement of SSE yet,
so we can't rely on __SSE__ being set at compile time.  To use SSE
code we need to put it in a separate GYP target compiled with -msse.

This patch set does exactly that for vector_fmac::FMAC_SSE() and
SincResampler::Convolve_SSE().  Doing so required some slight
rearrangements of constants for SincResampler.

Given all of our bots should have SSE I've made it a requirement
for passing the tests when run on X86.

BUG=none
TEST=media_unittests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=186285

[DaleCurtis, 2013-03-05 21:05:26]

cc: mark since he kindly offered to review.

[Mark Mentovai, 2013-03-05 21:32:31]

This is exactly what I had in mind (with the further optimization that you don’t
ever need to call the _C options on Mac).

https://codereview.chromium.org/12478002/diff/9001/media/base/simd/vector_mat...
File media/base/simd/vector_math_sse.cc (right):

https://codereview.chromium.org/12478002/diff/9001/media/base/simd/vector_mat...
media/base/simd/vector_math_sse.cc:7: #include <xmmintrin.h>  // NOLINT
Is NOLINT needed for an #include line? You didn’t write NOLINT on other
#includes of <xmmintrin.h>.

https://codereview.chromium.org/12478002/diff/9001/media/base/sinc_resampler.cc
File media/base/sinc_resampler.cc (right):

https://codereview.chromium.org/12478002/diff/9001/media/base/sinc_resampler....
media/base/sinc_resampler.cc:218: static const ConvolveProc kConvolveProc =
#if defined(OS_MACOSX), you can use Convolve_SSE directly, with no need for
base::CPU().

https://codereview.chromium.org/12478002/diff/9001/media/base/sinc_resampler.h
File media/base/sinc_resampler.h (right):

https://codereview.chromium.org/12478002/diff/9001/media/base/sinc_resampler....
media/base/sinc_resampler.h:79: static float Convolve_SSE(const float*
input_ptr, const float* k1,
Can you wrap Convolve_SSE and Convolve_NEON in the appropriate #ifdefs to avoid
exposing the declarations on irrelevant architectures?

Same for FMAC_SSE in media/base/vector_math_testing.h.

https://codereview.chromium.org/12478002/diff/9001/media/base/vector_math.cc
File media/base/vector_math.cc (right):

https://codereview.chromium.org/12478002/diff/9001/media/base/vector_math.cc#...
media/base/vector_math.cc:25: static const VectorFMACProc kVectorFMACProc =
#if defined(OS_MACOSX), you can use FMAC_SSE directly, with no need for
base::CPU().

https://codereview.chromium.org/12478002/diff/9001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/12478002/diff/9001/media/media.gyp#newcode1627
media/media.gyp:1627: # OSX already enables SSE2 or higher.
…and OSX doesn’t use cflags either, so this would be pointless.

[DaleCurtis, 2013-03-05 21:51:37]

https://codereview.chromium.org/12478002/diff/9001/media/base/simd/vector_mat...
File media/base/simd/vector_math_sse.cc (right):

https://codereview.chromium.org/12478002/diff/9001/media/base/simd/vector_mat...
media/base/simd/vector_math_sse.cc:7: #include <xmmintrin.h>  // NOLINT
On 2013/03/05 21:32:31, Mark Mentovai wrote:
> Is NOLINT needed for an #include line? You didn’t write NOLINT on other
> #includes of <xmmintrin.h>.

PRESUBMIT doesn't recognize "vector_math_testing" as a valid main include since
"vector_math_sse" is not a subset of "vector_math_testing", but "sinc_resampler"
work since it is a subset of "sinc_resampler_sse".

https://code.google.com/p/chromium/codesearch#chromium/src/PRESUBMIT.py&l=544

https://codereview.chromium.org/12478002/diff/9001/media/base/sinc_resampler.cc
File media/base/sinc_resampler.cc (right):

https://codereview.chromium.org/12478002/diff/9001/media/base/sinc_resampler....
media/base/sinc_resampler.cc:218: static const ConvolveProc kConvolveProc =
On 2013/03/05 21:32:31, Mark Mentovai wrote:
> #if defined(OS_MACOSX), you can use Convolve_SSE directly, with no need for
> base::CPU().

Even better I can just check for __SSE__.

https://codereview.chromium.org/12478002/diff/9001/media/base/sinc_resampler.h
File media/base/sinc_resampler.h (right):

https://codereview.chromium.org/12478002/diff/9001/media/base/sinc_resampler....
media/base/sinc_resampler.h:79: static float Convolve_SSE(const float*
input_ptr, const float* k1,
On 2013/03/05 21:32:31, Mark Mentovai wrote:
> Can you wrap Convolve_SSE and Convolve_NEON in the appropriate #ifdefs to
avoid
> exposing the declarations on irrelevant architectures?
> 
> Same for FMAC_SSE in media/base/vector_math_testing.h.

Done.

https://codereview.chromium.org/12478002/diff/9001/media/base/vector_math.cc
File media/base/vector_math.cc (right):

https://codereview.chromium.org/12478002/diff/9001/media/base/vector_math.cc#...
media/base/vector_math.cc:25: static const VectorFMACProc kVectorFMACProc =
On 2013/03/05 21:32:31, Mark Mentovai wrote:
> #if defined(OS_MACOSX), you can use FMAC_SSE directly, with no need for
> base::CPU().

Done w/ __SSE__.

https://codereview.chromium.org/12478002/diff/9001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/12478002/diff/9001/media/media.gyp#newcode1627
media/media.gyp:1627: # OSX already enables SSE2 or higher.
On 2013/03/05 21:32:31, Mark Mentovai wrote:
> …and OSX doesn’t use cflags either, so this would be pointless.

Yes! I was irritated that this expression was necessary as it's linking concepts
expressed is other .gyp files.

[Mark Mentovai, 2013-03-05 22:06:35]

Good use of __SSE__. LGTM.

[scherkus (not reviewing), 2013-03-05 22:33:45]

lgtm

looks like you have a few busted bots

[DaleCurtis, 2013-03-05 22:41:57]

Yeah, iOS apparently sets target_arch=x86||x64 http://crbug.com/172682 and
Windows had issues with MEDIA_EXPORT.  Both were tiny fixes similar to those
used elsewhere.

[commit-bot: I haz the power, 2013-03-05 22:51:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/12478002/13002

[commit-bot: I haz the power, 2013-03-05 23:03:06]

Retried try job too often on ios_dbg_simulator for step(s) base_unittests,
compile, content_unittests, crypto_unittests, googleurl_unittests,
media_unittests, net_unittests, sql_unittests, ui_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[DaleCurtis, 2013-03-06 00:03:15]

Committed patchset #4 manually as r186285 (presubmit successful).