[MIPS] Added MIPS DSPr2 optimization for BGRAConvolve2D routine

Added MIPS DSPr2 optimization for BGRAConvolve2D routine.

The following routines are optimized:
 - ConvolveVertically
 - ConvolveHorizontally

Performance gain measured on Malta 74Kc board:
 - our standalone test/bench application ~45%
 - chromium unit_test ~20%

R=hubbe@chromium.org, senorblanco@chromium.org

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

[Teodora Novkovic, 2013-05-10 15:47:23]

Added MIPS DSPr2 optimization for BGRAConvolve2D routine

[hubbe, 2013-05-10 19:39:08]

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc
File skia/ext/convolver.cc (right):

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc#newcode353
skia/ext/convolver.cc:353: bool has_alpha);
Just add "has_alpha" to all implementations instead of creating a special
typedef for MIPS.

[Stephen White, 2013-05-10 20:31:26]

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc
File skia/ext/convolver.cc (right):

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc#newcode353
skia/ext/convolver.cc:353: bool has_alpha);
On 2013/05/10 19:39:09, hubbe wrote:
> Just add "has_alpha" to all implementations instead of creating a special
> typedef for MIPS.

+1 to that.

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc#newcode379
skia/ext/convolver.cc:379: #elif defined SIMD_MIPS_DSPR2
You use "#if" above, but "#if defined" here.  Please choose one (preferably #if,
since it matches SIMD_SSE2).

[Teodora Novkovic, 2013-05-13 06:47:23]

Added "has_alpha" to all implementations and use #if instead of #if defined

[Stephen White, 2013-05-13 14:08:43]

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc
File skia/ext/convolver.cc (right):

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc#newcode379
skia/ext/convolver.cc:379: #elif defined SIMD_MIPS_DSPR2
On 2013/05/10 20:31:26, Stephen White wrote:
> You use "#if" above, but "#if defined" here.  Please choose one (preferably
#if,
> since it matches SIMD_SSE2).

Sorry, my mistake.  This should be #elif defined(SIMD_MIPS_DSPR2).  It's the
other ones that were wrong (and are now removed).

https://codereview.chromium.org/14929006/diff/6001/skia/ext/convolver_SSE2.cc
File skia/ext/convolver_SSE2.cc (right):

https://codereview.chromium.org/14929006/diff/6001/skia/ext/convolver_SSE2.cc...
skia/ext/convolver_SSE2.cc:17: template<bool has_alpha>
If this function is going to ignore has_alpha anyway, I'd just add it as a
non-templated parameter and get rid of the caller below.

[Teodora Novkovic, 2013-05-13 16:23:06]

Add a non-templated parameter "has_alpha" in ConvolveHorizontally_SSE2 routine

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc
File skia/ext/convolver.cc (right):

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc#newcode379
skia/ext/convolver.cc:379: #elif defined SIMD_MIPS_DSPR2
On 2013/05/13 14:08:43, Stephen White wrote:
> On 2013/05/10 20:31:26, Stephen White wrote:
> > You use "#if" above, but "#if defined" here.  Please choose one (preferably
> #if,
> > since it matches SIMD_SSE2).
> 
> Sorry, my mistake.  This should be #elif defined(SIMD_MIPS_DSPR2).  It's the
> other ones that were wrong (and are now removed).

Done.

https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc#newcode379
skia/ext/convolver.cc:379: #elif defined SIMD_MIPS_DSPR2
On 2013/05/10 20:31:26, Stephen White wrote:
> You use "#if" above, but "#if defined" here.  Please choose one (preferably
#if,
> since it matches SIMD_SSE2).

Done.

https://codereview.chromium.org/14929006/diff/6001/skia/ext/convolver_SSE2.cc
File skia/ext/convolver_SSE2.cc (right):

https://codereview.chromium.org/14929006/diff/6001/skia/ext/convolver_SSE2.cc...
skia/ext/convolver_SSE2.cc:17: template<bool has_alpha>
On 2013/05/13 14:08:43, Stephen White wrote:
> If this function is going to ignore has_alpha anyway, I'd just add it as a
> non-templated parameter and get rid of the caller below.

Done.

[Stephen White, 2013-05-13 17:00:31]

On 2013/05/13 16:23:06, Teodora Novkovic wrote:
> Add a non-templated parameter "has_alpha" in ConvolveHorizontally_SSE2 routine
> 
> https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc
> File skia/ext/convolver.cc (right):
> 
>
https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc#newcode379
> skia/ext/convolver.cc:379: #elif defined SIMD_MIPS_DSPR2
> On 2013/05/13 14:08:43, Stephen White wrote:
> > On 2013/05/10 20:31:26, Stephen White wrote:
> > > You use "#if" above, but "#if defined" here.  Please choose one
(preferably
> > #if,
> > > since it matches SIMD_SSE2).
> > 
> > Sorry, my mistake.  This should be #elif defined(SIMD_MIPS_DSPR2).  It's the
> > other ones that were wrong (and are now removed).
> 
> Done.
> 
>
https://codereview.chromium.org/14929006/diff/1/skia/ext/convolver.cc#newcode379
> skia/ext/convolver.cc:379: #elif defined SIMD_MIPS_DSPR2
> On 2013/05/10 20:31:26, Stephen White wrote:
> > You use "#if" above, but "#if defined" here.  Please choose one (preferably
> #if,
> > since it matches SIMD_SSE2).
> 
> Done.
> 
> https://codereview.chromium.org/14929006/diff/6001/skia/ext/convolver_SSE2.cc
> File skia/ext/convolver_SSE2.cc (right):
> 
>
https://codereview.chromium.org/14929006/diff/6001/skia/ext/convolver_SSE2.cc...
> skia/ext/convolver_SSE2.cc:17: template<bool has_alpha>
> On 2013/05/13 14:08:43, Stephen White wrote:
> > If this function is going to ignore has_alpha anyway, I'd just add it as a
> > non-templated parameter and get rid of the caller below.
> 
> Done.

LGTM

[hubbe, 2013-05-13 17:14:35]

lgtm

[Teodora Novkovic, 2013-05-14 12:35:15]

On 2013/05/13 17:14:35, hubbe wrote:
> lgtm

It seems that the "commit" check box does not work properly. Could you please
commit these changes for me? Thanks

[Stephen White, 2013-05-14 13:48:55]

On 2013/05/14 12:35:15, Teodora Novkovic wrote:
> On 2013/05/13 17:14:35, hubbe wrote:
> > lgtm
> 
> It seems that the "commit" check box does not work properly. Could you please
> commit these changes for me? Thanks

It has to be clicked by a committer. I've re-checked it.

[Teodora Novkovic, 2013-05-14 14:10:41]

On 2013/05/14 13:48:55, Stephen White wrote:
> On 2013/05/14 12:35:15, Teodora Novkovic wrote:
> > On 2013/05/13 17:14:35, hubbe wrote:
> > > lgtm
> > 
> > It seems that the "commit" check box does not work properly. Could you
please
> > commit these changes for me? Thanks
> 
> It has to be clicked by a committer. I've re-checked it.

I've checked it again, but nothing happens. There are no any message.

[Teodora Novkovic, 2013-05-17 12:51:34]

Commit Queue still does not work. Could you please help me with this problem?

[Stephen White, 2013-05-17 17:29:53]

On 2013/05/17 12:51:34, Teodora Novkovic wrote:
> Commit Queue still does not work. Could you please help me with this problem?

I'll land it.

[Teodora Novkovic, 2013-05-17 19:21:28]

On 2013/05/17 17:29:53, Stephen White wrote:
> On 2013/05/17 12:51:34, Teodora Novkovic wrote:
> > Commit Queue still does not work. Could you please help me with this
problem?
> 
> I'll land it.

Thanks

[Stephen White, 2013-05-17 21:36:47]

Committed patchset #3 manually as r200891 (presubmit successful).

[Stephen White, 2013-05-17 22:14:40]

Unfortunately this patch had to be reverted due to a missing license header.

Also, please ensure that you have signed the contributor license agreement, so
we can accept your contribution to Chrome.

The CLA is at https://developers.google.com/open-source/cla/individual (for
individuals) or https://developers.google.com/open-source/cla/corporate (for
corporations).

https://codereview.chromium.org/14929006/diff/16001/skia/ext/convolver_mips_d...
File skia/ext/convolver_mips_dspr2.cc (right):

https://codereview.chromium.org/14929006/diff/16001/skia/ext/convolver_mips_d...
skia/ext/convolver_mips_dspr2.cc:1: #include <algorithm>
This file needs a license header.

[Teodora Novkovic, 2013-05-18 20:33:30]

License header is added to convolver_mips_dspr2.cc file and also I have signed
the contributor license agreement.

[Teodora Novkovic, 2013-05-20 07:13:34]

Commit Queue still does not work and I would ask you to commit changes for me
again. Thanks

[Stephen White, 2013-05-21 18:12:55]

On 2013/05/18 20:33:30, Teodora Novkovic wrote:
> License header is added to convolver_mips_dspr2.cc file and also I have signed
> the contributor license agreement.

Hmm, I'm not seeing it in our CLA signers. Did you sign the CLA with the same
address that this patch is uploaded from? (teodora.petrovic@gmail.com)?

If not, you'll either have to re-sign the CLA with that address, or re-upload
your patch in a new codereview with the CLA-signed address.

Apologies for the silly hurdles: our legal requirements are such the addresses
must match.

[Stephen White, 2013-05-21 18:13:20]

On 2013/05/20 07:13:34, Teodora Novkovic wrote:
> Commit Queue still does not work and I would ask you to commit changes for me
> again. Thanks

CLA signing is orthogonal to achieving Chromium committer status (and ability to
use the commit queue).

[Teodora Novkovic, 2013-05-22 06:31:09]

Sorry, my mistake (I have used imgtec mail for CLA). I have signed again with
teodora.petrovic@gmail.com, so please check if everything is OK now. Thanks.

[Stephen White, 2013-05-22 15:46:10]

https://codereview.chromium.org/14929006/diff/39001/skia/ext/convolver_mips_d...
File skia/ext/convolver_mips_dspr2.cc (right):

https://codereview.chromium.org/14929006/diff/39001/skia/ext/convolver_mips_d...
skia/ext/convolver_mips_dspr2.cc:1: // Copyright (c) 2012 The Chromium Authors.
All rights reserved.
Nit: 2013 (here and elsewhere). Will fix on landing.

[Stephen White, 2013-05-22 15:46:24]

On 2013/05/22 06:31:09, Teodora Novkovic wrote:
> Sorry, my mistake (I have used imgtec mail for CLA). I have signed again with
> mailto:teodora.petrovic@gmail.com, so please check if everything is OK now.
Thanks.

Got it. Will give it another try today.

[Stephen White, 2013-05-23 19:14:38]

On 2013/05/22 15:46:24, Stephen White wrote:
> On 2013/05/22 06:31:09, Teodora Novkovic wrote:
> > Sorry, my mistake (I have used imgtec mail for CLA). I have signed again
with
> > mailto:teodora.petrovic@gmail.com, so please check if everything is OK now.
> Thanks.
> 
> Got it. Will give it another try today.

Could you take a look at Justin's comments over on
https://codereview.chromium.org/15742005/ ? You can upload your changes here,
and I'll copy them over there to try to appease the commit queue.

[Teodora Novkovic, 2013-05-24 16:01:54]

Add a non-templated parameter "has_alpha" for MIPS DSPr2 routines

[Stephen White, 2013-05-28 19:41:48]

Landed over on https://codereview.chromium.org/15742005/ as
http://crrev.com/202197; closing this issue.