ARM Skia NEON patches - 29 - Xfermode: SkFourByteInterp

ARM Skia NEON patches - 29 - Xfermode: SkFourByteInterp

Xfermode: add a NEON version of SkFourByteInterp

Brings a modest performance improvement on its own in
ProcXfermodes when aa is neither zero nor FF. Combined
with 1-pixel NEON modeprocs, it brings up to 35% speed
improvement on the aa case.

Signed-off-by: Kévin PETIT <kevin.petit@arm.com>;

BUG=

Committed: http://code.google.com/p/skia/source/detail?r=12448

[kevin.petit.not.used.account, 2013-12-02 15:39:26]

ping.

This one should be fairly non-controversial. It has no mismatches and brings a
modest speed improvement on its own (5 to 10% IIRC) for very little code.

It is also the opportunity for me to discuss the aa case in Xfermodes. At first
I tried to implement a full NEON version of the aa case but even if the NEON
code is a big win in many situations it just cannot compete when aa is all zeros
(all alternatives have to be calculated using NEON). Is that latter case common?
Could the full NEON version be of any interest?

[reed1, 2013-12-02 16:25:42]

lgtm w/ comment nits

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkColor_opts_neon.h
File src/opts/SkColor_opts_neon.h (right):

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkColor_opts_neon...
src/opts/SkColor_opts_neon.h:86: unsigned scale) {
Can we add SkASSERT(scale <= 256); at the beginning, just to help document the
subtle difference between this guy and the next.

Perhaps rename scale to srcWeight or srcScale, for clarity?

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkColor_opts_neon...
src/opts/SkColor_opts_neon.h:107: U8CPU srcWeight) {
SkASSERT(srcWeight <= 255);

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkXfermode_opts_a...
File src/opts/SkXfermode_opts_arm_neon.cpp (right):

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkXfermode_opts_a...
src/opts/SkXfermode_opts_arm_neon.cpp:634: if (a != 0xFF) {
Does this test/branch help in the neon case? Just wondering.

[kevin.petit.not.used.account, 2013-12-02 16:59:44]

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkColor_opts_neon.h
File src/opts/SkColor_opts_neon.h (right):

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkColor_opts_neon...
src/opts/SkColor_opts_neon.h:86: unsigned scale) {
On 2013/12/02 16:25:42, reed1 wrote:
> Can we add SkASSERT(scale <= 256); at the beginning, just to help document the
> subtle difference between this guy and the next.
> 
> Perhaps rename scale to srcWeight or srcScale, for clarity?

Done.

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkColor_opts_neon...
src/opts/SkColor_opts_neon.h:107: U8CPU srcWeight) {
On 2013/12/02 16:25:42, reed1 wrote:
> SkASSERT(srcWeight <= 255);

Done.

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkXfermode_opts_a...
File src/opts/SkXfermode_opts_arm_neon.cpp (right):

https://codereview.chromium.org/23724013/diff/8001/src/opts/SkXfermode_opts_a...
src/opts/SkXfermode_opts_arm_neon.cpp:634: if (a != 0xFF) {
On 2013/12/02 16:25:42, reed1 wrote:
> Does this test/branch help in the neon case? Just wondering.

For this change: yes.

If you were thinking of the full NEON case (which is not that change): nope.
It's the even the contrary though honestly it has a small impact compared to the
"0 != a" test just above it. The more if/else, the slower the NEON code will be.

For example, assume a code like this one:

if (want_to_do_it) {
    if (foo)
        result = fn1(data);
    else
        result = fn2(data);
}

When unrolling such a code in a loop using NEON, you have to compute fn1(data)
AND fn2(data) even if want_to_do_it is false for all the input data. ARMv8
allows to special-case in the latter case but at the expense of a slight cost.

[commit-bot: I haz the power, 2013-12-02 22:24:27]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/kevin.petit.arm@gmail.com/23724013/28001

[commit-bot: I haz the power, 2013-12-02 22:40:57]

Change committed as 12448