Fix memcpy32_sse2_unalign.

Fix memcpy32_sse2_unalign.

The whole point of mempcy32_sse2_unalign is that we didn't align dst128
and src128.  So it's not safe at all to cast them back to dst and src.
That tells the compiler that dst/src are 128-bit aligned, and then it
autovectorizes the cleanup while-loop using that (false) knowledge with
aligned SSE instructions.

This leads to crashes on memcpy32_sse2_unalign_10, which is small enough
that we actually get non-16-byte aligned memory.  The larger size
benches could be crashing too, but they're big enough allocations that
they're probably always 16-byte aligned anyway.

BUG=skia:2589
Committed: http://code.google.com/p/skia/source/detail?r=14851

[mtklein, 2014-05-22 16:38:09]

We're waiting on try bots, but I'm pretty hopeful I've got a good model now.

[f(malita), 2014-05-22 16:46:34]

lgtm

https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp
File bench/MemcpyBench.cpp (right):

https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp#newcod...
bench/MemcpyBench.cpp:137: src += 16;
Is the compiler smart enough to hoist these?

[mtklein, 2014-05-22 16:48:59]

https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp
File bench/MemcpyBench.cpp (right):

https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp#newcod...
bench/MemcpyBench.cpp:137: src += 16;
On 2014/05/22 16:46:34, Florin Malita wrote:
> Is the compiler smart enough to hoist these?

Probably.  I was worried that _I_ wasn't smart enough to hoist them correctly. 
Would it have been,

dst += 16 * (count / 16);
src += 16 * (count / 16);

?

[f(malita), 2014-05-22 16:51:43]

On 2014/05/22 16:48:59, mtklein wrote:
> https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp
> File bench/MemcpyBench.cpp (right):
> 
>
https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp#newcod...
> bench/MemcpyBench.cpp:137: src += 16;
> On 2014/05/22 16:46:34, Florin Malita wrote:
> > Is the compiler smart enough to hoist these?
> 
> Probably.  I was worried that _I_ wasn't smart enough to hoist them correctly.

> Would it have been,
> 
> dst += 16 * (count / 16);
> src += 16 * (count / 16);
> 
> ?

I *think* so :)

[mtklein, 2014-05-22 16:59:02]

https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp
File bench/MemcpyBench.cpp (right):

https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp#newcod...
bench/MemcpyBench.cpp:137: src += 16;
On 2014/05/22 16:48:59, mtklein wrote:
> On 2014/05/22 16:46:34, Florin Malita wrote:
> > Is the compiler smart enough to hoist these?
> 
> Probably.  I was worried that _I_ wasn't smart enough to hoist them correctly.

> Would it have been,
> 
> dst += 16 * (count / 16);
> src += 16 * (count / 16);
> 
> ?

Done.

While I was looking at it, I realized that, if I'm right, then the _align
version was half-broken too: src128 isn't aligned (dst128 is).  Made the same
change there.

[mtklein, 2014-05-22 17:19:01]

On 2014/05/22 16:59:02, mtklein wrote:
> https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp
> File bench/MemcpyBench.cpp (right):
> 
>
https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp#newcod...
> bench/MemcpyBench.cpp:137: src += 16;
> On 2014/05/22 16:48:59, mtklein wrote:
> > On 2014/05/22 16:46:34, Florin Malita wrote:
> > > Is the compiler smart enough to hoist these?
> > 
> > Probably.  I was worried that _I_ wasn't smart enough to hoist them
correctly.
> 
> > Would it have been,
> > 
> > dst += 16 * (count / 16);
> > src += 16 * (count / 16);
> > 
> > ?
> 
> Done.
> 
> While I was looking at it, I realized that, if I'm right, then the _align
> version was half-broken too: src128 isn't aligned (dst128 is).  Made the same
> change there.

I won't rule out being right until the trybot comes back, but I'm getting less
confident.  I can't see any problems with the generated code when I build head
with GCC 4.6-4.8 or Clang, in both 32 and 64 bit modes.  That said, I was never
able to reproduce this locally.

[mtklein, 2014-05-22 18:16:41]

On 2014/05/22 17:19:01, mtklein wrote:
> On 2014/05/22 16:59:02, mtklein wrote:
> > https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp
> > File bench/MemcpyBench.cpp (right):
> > 
> >
>
https://codereview.chromium.org/291893008/diff/1/bench/MemcpyBench.cpp#newcod...
> > bench/MemcpyBench.cpp:137: src += 16;
> > On 2014/05/22 16:48:59, mtklein wrote:
> > > On 2014/05/22 16:46:34, Florin Malita wrote:
> > > > Is the compiler smart enough to hoist these?
> > > 
> > > Probably.  I was worried that _I_ wasn't smart enough to hoist them
> correctly.
> > 
> > > Would it have been,
> > > 
> > > dst += 16 * (count / 16);
> > > src += 16 * (count / 16);
> > > 
> > > ?
> > 
> > Done.
> > 
> > While I was looking at it, I realized that, if I'm right, then the _align
> > version was half-broken too: src128 isn't aligned (dst128 is).  Made the
same
> > change there.
> 
> I won't rule out being right until the trybot comes back, but I'm getting less
> confident.  I can't see any problems with the generated code when I build head
> with GCC 4.6-4.8 or Clang, in both 32 and 64 bit modes.  That said, I was
never
> able to reproduce this locally.

So, looks like the try bot is going to pass.  Haven't been able to reproduce
this ever on my desktop.  I'm going to submit this.

I've been investigating skia:2575 while we wait.  I'm still pretty suspicious
that these must be two manifestations of the same issue.  Over there, I _can_
now reproduce the bug, but haven't pinned it down yet.  Best I know is that it's
only happening when I use gcc 4.6.  4.7 and 4.8 are fine.  So, maybe it's a
compiler bug that we need to upgrade past?  I think this Alex builder and the
x86 Release builder that's failing are both using gcc 4.6.

[mtklein, 2014-05-22 18:17:39]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-05-22 18:18:33]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/291893008/20001

[commit-bot: I haz the power, 2014-05-22 18:24:54]

Change committed as 14851