Sk4px

Sk4px

Xfermode_SrcOver:
   SSE: 2.08ms -> 2.03ms  (~2% faster)
   NEON: my N5 is noisy, but there appears to be no perf change

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/d2ffd36eb62e99abe2920369d1e040954cc2044f

[mtklein, 2015-05-11 19:06:54]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-11 19:07:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1132273004/70001

[commit-bot: I haz the power, 2015-05-11 19:08:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-11 19:08:28]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)

[mtklein, 2015-05-11 19:19:23]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-11 19:19:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1132273004/90001

[commit-bot: I haz the power, 2015-05-11 19:21:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-11 19:21:43]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)

[mtklein, 2015-05-11 19:33:26]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-11 19:33:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1132273004/110001

[commit-bot: I haz the power, 2015-05-11 19:39:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-11 19:39:29]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-05-11 21:24:27]

mtklein@google.com changed reviewers:
+ reed@google.com

[mtklein, 2015-05-11 22:19:13]

I just took a closer look: the invA unpacking _is_ being hoisted out of the
loop.  Faith in optimizer is restored.

It looks like the 3-4% slowdown on SSE is due to different instruction ordering,
e.g.

HEAD:
+0x1ad      punpcklbw           %xmm2, %xmm3
+0x1b1      pmullw              %xmm1, %xmm3
+0x1b5      punpckhbw           %xmm2, %xmm5
+0x1b9      pmullw              %xmm1, %xmm5
+0x1bd      paddw               %xmm0, %xmm3
+0x1c1      psrlw               $8, %xmm3
+0x1c6      paddw               %xmm0, %xmm5
+0x1ca      psrlw               $8, %xmm5
+0x1cf      packuswb            %xmm5, %xmm3

Sk4px:
+0x1cd      punpcklbw           %xmm3, %xmm2
+0x1d1      punpckhbw           %xmm3, %xmm6
+0x1d5      pmullw              %xmm4, %xmm2
+0x1d9      pmullw              %xmm5, %xmm6
+0x1dd      paddw               %xmm0, %xmm2
+0x1e1      paddw               %xmm1, %xmm6
+0x1e5      psrlw               $8, %xmm2
+0x1ea      psrlw               $8, %xmm6
+0x1ef      packuswb            %xmm6, %xmm2

This is a side effect of the way I've organized Sk4px.  Going to poke around a
bit to see if there's anything I can do about it, but I think this is probably
close enough to land and see how it performs on the bots.

[mtklein, 2015-05-11 23:13:02]

On 2015/05/11 22:19:13, mtklein wrote:
> I just took a closer look: the invA unpacking _is_ being hoisted out of the
> loop.  Faith in optimizer is restored.
> 
> It looks like the 3-4% slowdown on SSE is due to different instruction
ordering,
> e.g.
> 
> HEAD:
> +0x1ad      punpcklbw           %xmm2, %xmm3
> +0x1b1      pmullw              %xmm1, %xmm3
> +0x1b5      punpckhbw           %xmm2, %xmm5
> +0x1b9      pmullw              %xmm1, %xmm5
> +0x1bd      paddw               %xmm0, %xmm3
> +0x1c1      psrlw               $8, %xmm3
> +0x1c6      paddw               %xmm0, %xmm5
> +0x1ca      psrlw               $8, %xmm5
> +0x1cf      packuswb            %xmm5, %xmm3
> 
> Sk4px:
> +0x1cd      punpcklbw           %xmm3, %xmm2
> +0x1d1      punpckhbw           %xmm3, %xmm6
> +0x1d5      pmullw              %xmm4, %xmm2
> +0x1d9      pmullw              %xmm5, %xmm6
> +0x1dd      paddw               %xmm0, %xmm2
> +0x1e1      paddw               %xmm1, %xmm6
> +0x1e5      psrlw               $8, %xmm2
> +0x1ea      psrlw               $8, %xmm6
> +0x1ef      packuswb            %xmm6, %xmm2
> 
> This is a side effect of the way I've organized Sk4px.  Going to poke around a
> bit to see if there's anything I can do about it, but I think this is probably
> close enough to land and see how it performs on the bots.

*sigh*  All that was bogus.  The instruction order doesn't make a difference.

The hoisting out of the loop isn't quite the problem.  It _is_ hoisted out of
the main loop, but then repeated for the 4, 2, and 1 pixel tail sections each
where head only does that once.  Looks like I need to fiddle around with how
Sk4px::MapSrc works, not with the core blend code.

[mtklein, 2015-05-11 23:32:43]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-11 23:33:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1132273004/130001

[commit-bot: I haz the power, 2015-05-11 23:39:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-11 23:39:33]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-05-11 23:40:47]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-11 23:41:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1132273004/150001

[commit-bot: I haz the power, 2015-05-11 23:47:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-11 23:47:04]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-05-11 23:56:18]

Ho ho ho, great success.

I've rewritten MapSrc to allow for invariant hoisting across all calls to fn(),
and tweaked it a little bit more to actually net a small perf win on Intel.

This should be good to review/land now.

[reed1, 2015-05-12 13:10:24]

lgtm

[mtklein, 2015-05-12 13:10:49]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-05-12 13:11:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1132273004/150001

[commit-bot: I haz the power, 2015-05-12 13:11:28]

Committed patchset #9 (id:150001) as
https://skia.googlesource.com/skia/+/d2ffd36eb62e99abe2920369d1e040954cc2044f