Port SkMatrix opts to SkOpts.

Port SkMatrix opts to SkOpts.

No changes to the code, just moved around.

This will have the effect of enabling vectorized code on ARMv7.
Should be no effect on ARMv8 or x86, which would have been vectorized already.

nanobench --match mappoints changes on Nexus 5 (ARMv7):

_affine: 132 -> 95
_scale: 118 -> 47
_trans: 60 -> 37

A teaser:
We should next look at the ABCD->BADC shuffle we've noted that we need in _affine.  A quick hack showed doing that optimally is another ~35% speedup on x86.  Got to figure out how to do it best on ARM though: that same quick hack was a 2x slowdown there.  Good reason to resurrect that SkNx_shuffle() CL!

(I believe the answers are vrev64q_f32(v) and _mm_shuffle_ps(v,v, _MM_SHUFFLE(2,3,0,1), but we should probably find out in another CL.)

BUG=skia:4117
Committed: https://skia.googlesource.com/skia/+/4e8a09d3672702704436112f3aa5611bd79b2690

[mtklein, 2015-09-10 00:13:27]

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

[commit-bot: I haz the power, 2015-09-10 00:13:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1320673014/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1320673014/20001

[commit-bot: I haz the power, 2015-09-10 00:18:15]

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

[commit-bot: I haz the power, 2015-09-10 00:18:16]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-09-10 00:25:14]

Patchset #3 (id:40001) has been deleted

[mtklein_C, 2015-09-10 00:39:22]

mtklein@chromium.org changed reviewers:
+ reed@google.com
- mtklein@google.com

[reed1, 2015-09-10 16:51:19]

in the neon case (I guess), does this mean we jump to the C function, only to
jump into a neon version? I am presuming the opts versions don't get inlined.

If they don't, can we change how we build gMapPtsProcs[] so that I jump directly
to the "right" function each time, and skip the spring-board?

[mtklein, 2015-09-10 17:36:30]

On 2015/09/10 16:51:19, reed1 wrote:
> in the neon case (I guess), does this mean we jump to the C function, only to
> jump into a neon version? I am presuming the opts versions don't get inlined.
> 
> If they don't, can we change how we build gMapPtsProcs[] so that I jump
directly
> to the "right" function each time, and skip the spring-board?

Yes.  It's tricky.   You might think we could just write

SkMatrix::MapPointsProc gMapPtsProcs[] = { 
  SkMatrix::Identity_pts,  SkOpts::matrix_translate,
  SkOpts::matrix_scale_translate, SkOpts::matrix_scale_translate,
  SkOpts::matrix_affine, SkOpts::matrix_affine,
  SkOpts::matrix_affine, SkOpts::matrix_affine,
  ...
}

but that locks gMapPtsProcs[] into values of those SkOpts::matrix_... function
pointers as they were initialized at link time, the default non-SIMD versions.
&SkMatrix::Identity_pts never changes, but the function SkOpts::matrix_translate
points to does.

I don't actually see any inlining anywhere, whether Clang, GCC, x86, or ARM. 
E.g. both the outer SkMatrix::Trans_pts() and the inner
SkOpts::matrix_translate() are being called via function pointers.  I think we
might want to write gMapPtsProcs as a function returning a function pointer, so
we can at least get that down to just one indirect call.

[mtklein, 2015-09-10 18:05:36]

On 2015/09/10 17:36:30, mtklein wrote:
> On 2015/09/10 16:51:19, reed1 wrote:
> > in the neon case (I guess), does this mean we jump to the C function, only
to
> > jump into a neon version? I am presuming the opts versions don't get
inlined.
> > 
> > If they don't, can we change how we build gMapPtsProcs[] so that I jump
> directly
> > to the "right" function each time, and skip the spring-board?
> 
> Yes.  It's tricky.   You might think we could just write
> 
> SkMatrix::MapPointsProc gMapPtsProcs[] = { 
>   SkMatrix::Identity_pts,  SkOpts::matrix_translate,
>   SkOpts::matrix_scale_translate, SkOpts::matrix_scale_translate,
>   SkOpts::matrix_affine, SkOpts::matrix_affine,
>   SkOpts::matrix_affine, SkOpts::matrix_affine,
>   ...
> }
> 
> but that locks gMapPtsProcs[] into values of those SkOpts::matrix_... function
> pointers as they were initialized at link time, the default non-SIMD versions.
> &SkMatrix::Identity_pts never changes, but the function
SkOpts::matrix_translate
> points to does.
> 
> I don't actually see any inlining anywhere, whether Clang, GCC, x86, or ARM. 
> E.g. both the outer SkMatrix::Trans_pts() and the inner
> SkOpts::matrix_translate() are being called via function pointers.  I think we
> might want to write gMapPtsProcs as a function returning a function pointer,
so
> we can at least get that down to just one indirect call.

I tried out two similar alternatives (PS 4 and PS 5), but the version in PS2+3
with the trampolining is fastest.

[reed1, 2015-09-10 18:07:25]

I'm fine if you still want to land this as is, and consider a different round of
measurements/optimizations around function-overhead. LGTM

*If* we change that guy to a function call, perhaps it will make it easy to
specialize even more (e.g. based on number of points). Not sure if we need that
much generality, but we could at least consider it.

[mtklein, 2015-09-10 18:11:27]

On 2015/09/10 18:07:25, reed1 wrote:
> I'm fine if you still want to land this as is, and consider a different round
of
> measurements/optimizations around function-overhead. LGTM
SGTM.  Going to land ps3.

> *If* we change that guy to a function call, perhaps it will make it easy to
> specialize even more (e.g. based on number of points). Not sure if we need
that
> much generality, but we could at least consider it.

Right, certainly passing N doesn't hurt if we've got it.

[mtklein, 2015-09-10 18:11:34]

Patchset #5 (id:100001) has been deleted

[mtklein, 2015-09-10 18:11:39]

Patchset #4 (id:80001) has been deleted

[mtklein, 2015-09-10 18:12:02]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-09-10 18:12:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1320673014/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1320673014/60001

[commit-bot: I haz the power, 2015-09-10 18:18:34]

Committed patchset #3 (id:60001) as
https://skia.googlesource.com/skia/+/4e8a09d3672702704436112f3aa5611bd79b2690