ARM Skia NEON patches - 14 - S32A_Blend

ARM Skia NEON patches - 14 - S32A_Blend





Blitrow32: S32A_Blend new NEON version

Adding a NEON version of S32A_Blend_BlitRow32. Here are the
benchmark results:

+-------+--------------------------+--------------------------+
|       |      Speedup vs. C       |    Speedup vs. ARM asm   |
| count +------------+-------------+------------+-------------+
|       | Cortex A-9 | Cortex A-15 | Cortex A-9 | Cortex A-15 |
+-------+------------+-------------+------------+-------------+
| 1     | +8,5%      | +18,5%      | +0.9%      | +2,9%       |
+-------+------------+-------------+------------+-------------+
| 2     | +65,6%     | +94%        | +70,3%     | +80%        |
+-------+------------+-------------+------------+-------------+
| 4     | +42,4%     | +87,8%      | +56,8%     | +84,4%      |
+-------+------------+-------------+------------+-------------+
| 8     | +30%       | +90%        | +49,9%     | +82,7%      |
+-------+------------+-------------+------------+-------------+
| 16    | +23,1%     | +95,4%      | +46,6%     | +87,6%      |
+-------+------------+-------------+------------+-------------+
| 64    | +23,1%     | +95,7%      | +46,1%     | +89,4%      |
+-------+------------+-------------+------------+-------------+
| 256   | +35,5%     | +122%       | +53,6%     | +99,2%      |
+-------+------------+-------------+------------+-------------+
| 1024  | +61,8%     | +101%       | +64,2%     | +91,2%      |
+-------+------------+-------------+------------+-------------+

BUG=

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

[djsollen, 2013-07-12 19:45:40]

https://codereview.chromium.org/18614010/diff/1/src/opts/SkBlitRow_opts_arm_n...
File src/opts/SkBlitRow_opts_arm_neon.cpp (left):

https://codereview.chromium.org/18614010/diff/1/src/opts/SkBlitRow_opts_arm_n...
src/opts/SkBlitRow_opts_arm_neon.cpp:1297: S32A_Blend_BlitRow32_arm        //
S32A_Blend
we should remove the extern definition in SkBlitRow_opts_arm.h and the comment
in the SkBlitRow_opts_arm.cpp that states it is used by the neon version.

[djsollen, 2013-07-12 19:52:09]

your results seem to show that the C code is faster than the ASM. Is that your
finding as well?

[djsollen, 2013-07-12 19:53:40]

On 2013/07/12 19:52:09, djsollen wrote:
> your results seem to show that the C code is faster than the ASM. Is that your
> finding as well?

Actually, I think I was reading your chart incorrectly, but feel free to correct
me if I'm wrong.

[kevin.petit.not.used.account, 2013-07-15 09:08:49]

On 2013/07/12 19:53:40, djsollen wrote:
> On 2013/07/12 19:52:09, djsollen wrote:
> > your results seem to show that the C code is faster than the ASM. Is that
your
> > finding as well?
> 
> Actually, I think I was reading your chart incorrectly, but feel free to
correct
> me if I'm wrong.

In some cases, yes, the asm *is* significantly slower :-). It has probably been
written for older processors and/or gcc has improved in between.

[kevin.petit.not.used.account, 2013-07-30 13:27:53]

ping

[djsollen, 2013-07-31 13:49:41]

I got similar perf numbers when running on my devices as well, so it looks good
from that perspective.  However, we do need to clean up the warnings generated
by the trybots before we submit it.

[kevin.petit.not.used.account, 2013-07-31 14:44:22]

On 2013/07/31 13:49:41, djsollen wrote:
> I got similar perf numbers when running on my devices as well, so it looks
good
> from that perspective.  However, we do need to clean up the warnings generated
> by the trybots before we submit it.

What kind of warnings are you observing?

[djsollen, 2013-07-31 15:16:44]

click on the trybot results and follow it to the stdio of the broken build step.

[kevin.petit.not.used.account, 2013-07-31 15:32:06]

On 2013/07/31 15:16:44, djsollen wrote:
> click on the trybot results and follow it to the stdio of the broken build
step.

Thanks. My advice would be to initialise with "vdup_n_u8(0)" and see if it
doesn't affect the perf for count = 1. If it does, initialising with
"vreinterpret_u8_u32(vdup_n_u32(0))" should fix it.

[commit-bot: I haz the power, 2013-08-01 13:21:11]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[djsollen, 2013-08-01 13:21:28]

lgtm

[commit-bot: I haz the power, 2013-08-01 13:21:39]

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

[commit-bot: I haz the power, 2013-08-01 14:23:28]

Change committed as 10480