speed up rgn building by inlining memcmp for 32bit values

speed up rgn building by inlining memcmp for 32bit values

on mac/clang, using circularclips gm/bench

- before: 400ms
- after:  250ms

BUG=skia:

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

[Tom Hudson, 2014-06-20 18:50:05]

lgtm

https://codereview.chromium.org/348143002/diff/1/src/core/SkRegion_path.cpp
File src/core/SkRegion_path.cpp (right):

https://codereview.chromium.org/348143002/diff/1/src/core/SkRegion_path.cpp#n...
src/core/SkRegion_path.cpp:18: #if 0
Nit: would we want to replace 10 lines of dead code by a one-line comment ala
// grouping 4-at-a-time doesn't seem to speed this up

(But if it's ints, why were we expecting 4-at-a-time to speed this up? Automatic
SSE magic somewhere in the compiler? But there's not an alignment guarantee for
that, is there?)

https://codereview.chromium.org/348143002/diff/1/src/core/SkRegion_path.cpp#n...
src/core/SkRegion_path.cpp:36: return !memcmp(a, b, count << 2);
Nit: * sizeof(int32_t) is a little more intelligible than << 2 ?

[mtklein, 2014-06-20 19:20:38]

https://codereview.chromium.org/348143002/diff/1/src/core/SkRegion_path.cpp
File src/core/SkRegion_path.cpp (right):

https://codereview.chromium.org/348143002/diff/1/src/core/SkRegion_path.cpp#n...
src/core/SkRegion_path.cpp:18: #if 0
On 2014/06/20 18:50:05, Tom Hudson wrote:
> Nit: would we want to replace 10 lines of dead code by a one-line comment ala
> // grouping 4-at-a-time doesn't seem to speed this up
> 
> (But if it's ints, why were we expecting 4-at-a-time to speed this up?
Automatic
> SSE magic somewhere in the compiler? But there's not an alignment guarantee
for
> that, is there?)

We _could_ write this with SSE or Neon, and SSE wouldn't even strictly require
an alignment step.  E.g.
        __m128i a4 = _mm_loadu_si128(a);
        __m128i b4 = _mm_loadu_si128(b);
        __m128i eq = _mm_xor_si128(a4, b4);
        if (!_mm_testz_si128(eq, eq)) {
            return false;
        }
        a += 4;
        b += 4;

and I imagine the platform memcmp must be written that way.  That said, I can't
get either of Mike's implementations (#if 0 or #if 1) to auto vectorize.

This might make a good candidate to move to SkUtils so we can give it platform
versions.  Conceivably we could be memcmp, given that we only need to return
equal/not-equal, not the direction of the inequality.

[mtklein, 2014-06-20 19:22:07]

On 2014/06/20 19:20:38, mtklein wrote:
> https://codereview.chromium.org/348143002/diff/1/src/core/SkRegion_path.cpp
> File src/core/SkRegion_path.cpp (right):
> 
>
https://codereview.chromium.org/348143002/diff/1/src/core/SkRegion_path.cpp#n...
> src/core/SkRegion_path.cpp:18: #if 0
> On 2014/06/20 18:50:05, Tom Hudson wrote:
> > Nit: would we want to replace 10 lines of dead code by a one-line comment
ala
> > // grouping 4-at-a-time doesn't seem to speed this up
> > 
> > (But if it's ints, why were we expecting 4-at-a-time to speed this up?
> Automatic
> > SSE magic somewhere in the compiler? But there's not an alignment guarantee
> for
> > that, is there?)
> 
> We _could_ write this with SSE or Neon, and SSE wouldn't even strictly require
> an alignment step.  E.g.
>         __m128i a4 = _mm_loadu_si128(a);
>         __m128i b4 = _mm_loadu_si128(b);
>         __m128i eq = _mm_xor_si128(a4, b4);
>         if (!_mm_testz_si128(eq, eq)) {
>             return false;
>         }
>         a += 4;
>         b += 4;
> 
> and I imagine the platform memcmp must be written that way.  That said, I
can't
> get either of Mike's implementations (#if 0 or #if 1) to auto vectorize.
> 
> This might make a good candidate to move to SkUtils so we can give it platform
> versions.  Conceivably we could be memcmp, given that we only need to return
> equal/not-equal, not the direction of the inequality.

*beat memcmp

[Tom Hudson, 2014-06-20 19:23:08]

On 2014/06/20 19:20:38, mtklein wrote:
> This might make a good candidate to move to SkUtils so we can give it platform
> versions.  Conceivably we could be memcmp, given that we only need to return
> equal/not-equal, not the direction of the inequality.

I wasn't going to advocate for that until we found a second or third file that
needed this code, but then, yeah.

[reed1, 2014-06-20 20:44:21]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-06-20 20:45:24]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/reed@google.com/348143002/20001

[commit-bot: I haz the power, 2014-06-20 20:59:19]

Change committed as b8f07988494a62fbe8fba70129b1bb9366e9f6ee