Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(44)

Issue 302283003: ARM Skia NEON patches - 38 - arm64 8888 blitters (Closed)

Created:
6 years, 6 months ago by kevin.petit
Modified:
6 years, 6 months ago
Reviewers:
djsollen, mtklein
CC:
skia-review_googlegroups.com
Base URL:
https://skia.googlesource.com/skia.git@master
Visibility:
Public.

Description

ARM Skia NEON patches - 38 - arm64 8888 blitters Enable NEON on arm64 for most 8888 blitters This patch enables NEON optimisation for the Color32, S32_Blend, S32A_Opaque blitters on arm64. Here are the perf improvements vs the existing code: Color32: ======== +-------+------------+------------+ | count | Cortex-A53 | Cortex-A57 | +-------+------------+------------+ | 1 | -2.39% | 23.78% | +-------+------------+------------+ | 2 | -5.46% | 8.88% | +-------+------------+------------+ | 4 | -4.74% | 4.89% | +-------+------------+------------+ | 8 | 67.74% | 107.12% | +-------+------------+------------+ | 16 | 40.03% | 101.20% | +-------+------------+------------+ | 64 | 11.09% | 98.40% | +-------+------------+------------+ | 256 | -2.20% | 74.81% | +-------+------------+------------+ | 1024 | -4.28% | 78.90% | +-------+------------+------------+ S32_Blend: ========== +-------+------------+------------+ | count | Cortex-A53 | Cortex-A57 | +-------+------------+------------+ | 1 | 7.84% | -6.75% | +-------+------------+------------+ | 2 | 28.95% | 39.77% | +-------+------------+------------+ | 4 | 5.80% | 8.26% | +-------+------------+------------+ | 8 | 1.35% | 33.80% | +-------+------------+------------+ | 16 | -2.13% | 41.13% | +-------+------------+------------+ | 64 | -4.91% | 42.84% | +-------+------------+------------+ | 256 | -6.53% | 48.72% | +-------+------------+------------+ | 1024 | -6.65% | 46.66% | +-------+------------+------------+ S32A_Opaque: ============ +-------+------------+------------+ | count | Cortex-A53 | Cortex-A57 | +-------+------------+------------+ | 1 | -7.51% | -19.06% | +-------+------------+------------+ | 2 | -5.02% | -27.70% | +-------+------------+------------+ | 4 | 15.38% | -21.66% | +-------+------------+------------+ | 8 | -0.98% | 1.05% | +-------+------------+------------+ | 16 | -7.35% | 3.34% | +-------+------------+------------+ | 64 | 50.53% | 94.63% | +-------+------------+------------+ | 256 | 71.17% | 164.10% | +-------+------------+------------+ | 1024 | 79.58% | 197.60% | +-------+------------+------------+ Signed-off-by: Kevin PETIT <kevin.petit@arm.com>; BUG=skia: Committed: https://skia.googlesource.com/skia/+/866b95d65dfc01af372bbed206ec067e04c1f533

Patch Set 1 #

Total comments: 8

Patch Set 2 : Review comments #

Total comments: 4

Patch Set 3 : Comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -5 lines) Patch
M gyp/opts.gyp View 1 chunk +2 lines, -1 line 0 comments Download
M src/opts/SkBlitRow_opts_arm_neon.cpp View 1 2 9 chunks +17 lines, -4 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
kevin.petit
6 years, 6 months ago (2014-06-02 16:08:08 UTC) #1
mtklein
On 2014/06/02 16:08:08, kevin.petit wrote: Are we missing SkBlitRow_opts_arm.cpp here?
6 years, 6 months ago (2014-06-02 16:19:46 UTC) #2
kevin.petit
On 2014/06/02 16:19:46, mtklein wrote: > On 2014/06/02 16:08:08, kevin.petit wrote: > > Are we ...
6 years, 6 months ago (2014-06-02 16:23:30 UTC) #3
mtklein
Ah, I was confused. So we're just turning on existing opts now for aarch64 too? ...
6 years, 6 months ago (2014-06-02 18:19:49 UTC) #4
kevin.petit
>So we're just turning on existing opts now for aarch64 too? Yes. https://codereview.chromium.org/302283003/diff/1/src/opts/SkBlitRow_opts_arm_neon.cpp File src/opts/SkBlitRow_opts_arm_neon.cpp ...
6 years, 6 months ago (2014-06-03 09:30:52 UTC) #5
mtklein
ARM -> ARM32 sounds fine as another CL. https://codereview.chromium.org/302283003/diff/1/src/opts/SkBlitRow_opts_arm_neon.cpp File src/opts/SkBlitRow_opts_arm_neon.cpp (right): https://codereview.chromium.org/302283003/diff/1/src/opts/SkBlitRow_opts_arm_neon.cpp#newcode1414 src/opts/SkBlitRow_opts_arm_neon.cpp:1414: #else ...
6 years, 6 months ago (2014-06-03 16:01:04 UTC) #6
kevin.petit
https://codereview.chromium.org/302283003/diff/1/src/opts/SkBlitRow_opts_arm_neon.cpp File src/opts/SkBlitRow_opts_arm_neon.cpp (right): https://codereview.chromium.org/302283003/diff/1/src/opts/SkBlitRow_opts_arm_neon.cpp#newcode1414 src/opts/SkBlitRow_opts_arm_neon.cpp:1414: #else // (__GNUC__ > 4) || ((__GNUC__ == 4) ...
6 years, 6 months ago (2014-06-03 16:14:17 UTC) #7
mtklein
https://codereview.chromium.org/302283003/diff/20001/src/opts/SkBlitRow_opts_arm_neon.cpp File src/opts/SkBlitRow_opts_arm_neon.cpp (right): https://codereview.chromium.org/302283003/diff/20001/src/opts/SkBlitRow_opts_arm_neon.cpp#newcode1414 src/opts/SkBlitRow_opts_arm_neon.cpp:1414: #else // (__GNUC__ > 4) || ((__GNUC__ == 4) ...
6 years, 6 months ago (2014-06-03 16:17:19 UTC) #8
kevin.petit
https://codereview.chromium.org/302283003/diff/20001/src/opts/SkBlitRow_opts_arm_neon.cpp File src/opts/SkBlitRow_opts_arm_neon.cpp (right): https://codereview.chromium.org/302283003/diff/20001/src/opts/SkBlitRow_opts_arm_neon.cpp#newcode1414 src/opts/SkBlitRow_opts_arm_neon.cpp:1414: #else // (__GNUC__ > 4) || ((__GNUC__ == 4) ...
6 years, 6 months ago (2014-06-03 16:24:28 UTC) #9
mtklein
:) LGTM
6 years, 6 months ago (2014-06-03 16:33:10 UTC) #10
kevin.petit
The CQ bit was checked by kevin.petit@arm.com
6 years, 6 months ago (2014-06-03 16:57:31 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/kevin.petit@arm.com/302283003/40001
6 years, 6 months ago (2014-06-03 16:57:46 UTC) #12
commit-bot: I haz the power
6 years, 6 months ago (2014-06-03 17:08:12 UTC) #13
Message was sent while issue was closed.
Change committed as 866b95d65dfc01af372bbed206ec067e04c1f533

Powered by Google App Engine
This is Rietveld 408576698