Replace OS::MemCopy with OS::MemMove (just as fast but more flexible).

Replace OS::MemCopy with OS::MemMove (just as fast but more flexible).

Committed: http://code.google.com/p/v8/source/detail?r=14280

[Jakob Kummerow, 2013-04-12 13:23:10]

Vulcanizing more rubber. You can't have enough of that stuff.

Reviewer's guidance:
The meat of the change is in codegen-ia32.cc. There's some plumbing in
platform*.*, and tests in test-utils.cc. The rest is just mechanical
replacements at call sites.

[Michael Starzinger, 2013-04-16 09:41:42]

LGTM with a few comments.

https://codereview.chromium.org/13932006/diff/1/src/platform-posix.cc
File src/platform-posix.cc (right):

https://codereview.chromium.org/13932006/diff/1/src/platform-posix.cc#newcode330
src/platform-posix.cc:330: static OS::MemMoveFunction memmove_function =
&MemMoveWrapper;
nit: Add an empty newline after "memmove_function" declaration.

https://codereview.chromium.org/13932006/diff/1/src/platform-win32.cc
File src/platform-win32.cc (right):

https://codereview.chromium.org/13932006/diff/1/src/platform-win32.cc#newcode156
src/platform-win32.cc:156: static OS::MemMoveFunction memmove_function =
&MemMoveWrapper;
nit: Add an empty newline after "memmove_function" declaration.

https://codereview.chromium.org/13932006/diff/1/src/v8utils.h
File src/v8utils.h (right):

https://codereview.chromium.org/13932006/diff/1/src/v8utils.h#newcode186
src/v8utils.h:186: if (num_bytes >= static_cast<size_t>(kBlockCopyLimit)) {
nit: Can we also flip the predicate in this conditional so that the call to
OS::MemCopy is at the end, to be in sync with the other wrappers.

https://codereview.chromium.org/13932006/diff/1/src/v8utils.h#newcode207
src/v8utils.h:207: *reinterpret_cast<uint16_t*>(dst) = *reinterpret_cast<const
uint16_t*>(src);
On architectures that don't support unaligned access, this will trap and be
emulated by the kernel. Can we put all the unaligned cases (i.e. case 2 through
16) behind the V8_HOST_CAN_READ_UNALIGNED define?

[Jakob Kummerow, 2013-04-16 12:29:42]

Thanks for the review, comments addressed. Landing.

https://codereview.chromium.org/13932006/diff/1/src/platform-posix.cc
File src/platform-posix.cc (right):

https://codereview.chromium.org/13932006/diff/1/src/platform-posix.cc#newcode330
src/platform-posix.cc:330: static OS::MemMoveFunction memmove_function =
&MemMoveWrapper;
On 2013/04/16 09:41:42, Michael Starzinger wrote:
> nit: Add an empty newline after "memmove_function" declaration.

Done.

https://codereview.chromium.org/13932006/diff/1/src/platform-win32.cc
File src/platform-win32.cc (right):

https://codereview.chromium.org/13932006/diff/1/src/platform-win32.cc#newcode156
src/platform-win32.cc:156: static OS::MemMoveFunction memmove_function =
&MemMoveWrapper;
On 2013/04/16 09:41:42, Michael Starzinger wrote:
> nit: Add an empty newline after "memmove_function" declaration.

Done.

https://codereview.chromium.org/13932006/diff/1/src/v8utils.h
File src/v8utils.h (right):

https://codereview.chromium.org/13932006/diff/1/src/v8utils.h#newcode186
src/v8utils.h:186: if (num_bytes >= static_cast<size_t>(kBlockCopyLimit)) {
On 2013/04/16 09:41:42, Michael Starzinger wrote:
> nit: Can we also flip the predicate in this conditional so that the call to
> OS::MemCopy is at the end, to be in sync with the other wrappers.

Done.

https://codereview.chromium.org/13932006/diff/1/src/v8utils.h#newcode207
src/v8utils.h:207: *reinterpret_cast<uint16_t*>(dst) = *reinterpret_cast<const
uint16_t*>(src);
On 2013/04/16 09:41:42, Michael Starzinger wrote:
> On architectures that don't support unaligned access, this will trap and be
> emulated by the kernel. Can we put all the unaligned cases (i.e. case 2
through
> 16) behind the V8_HOST_CAN_READ_UNALIGNED define?

Done.

[Jakob Kummerow, 2013-04-16 12:31:41]

Committed patchset #2 manually as r14280 (presubmit successful).

[tfarina, 2013-04-17 01:41:00]

The CL summary/description is a bit misleading.

You changed a bunch of memcpy to OS::MemCopy specially in the first files of
this CL.

[Jakob Kummerow, 2013-04-17 07:49:40]

On 2013/04/17 01:41:00, tfarina wrote:
> The CL summary/description is a bit misleading.
>
> You changed a bunch of memcpy to OS::MemCopy specially in the first files of
> this CL.

So?