Commit 71bd1461 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

Fix overlap check in {CopyChars} and use {std::copy_n} unconditionally

This CL fixes the overlap check by using {<=} instead of {<}. This
allows us to always use {std::copy_n}, which should fall back to
{memcpy} internally (instead of the potentially slower {memmove} we
were using before).
This might also fix the regressions seen mostly on atom CPUs.

R=leszeks@chromium.org

Bug: chromium:1006157
Change-Id: Ib61048d65e99a9e7edac5ed894ceaf9e26ad4409
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1844781Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64131}
parent cf182b05
...@@ -198,8 +198,7 @@ inline void MemsetPointer(T** dest, U* value, size_t counter) { ...@@ -198,8 +198,7 @@ inline void MemsetPointer(T** dest, U* value, size_t counter) {
} }
// Copy from 8bit/16bit chars to 8bit/16bit chars. Values are zero-extended if // Copy from 8bit/16bit chars to 8bit/16bit chars. Values are zero-extended if
// needed. Ranges are not allowed to overlap unless the type sizes match (hence // needed. Ranges are not allowed to overlap.
// {memmove} is used internally).
// The separate declaration is needed for the V8_NONNULL, which is not allowed // The separate declaration is needed for the V8_NONNULL, which is not allowed
// on a definition. // on a definition.
template <typename SrcType, typename DstType> template <typename SrcType, typename DstType>
...@@ -210,27 +209,17 @@ void CopyChars(DstType* dst, const SrcType* src, size_t count) { ...@@ -210,27 +209,17 @@ void CopyChars(DstType* dst, const SrcType* src, size_t count) {
STATIC_ASSERT(std::is_integral<SrcType>::value); STATIC_ASSERT(std::is_integral<SrcType>::value);
STATIC_ASSERT(std::is_integral<DstType>::value); STATIC_ASSERT(std::is_integral<DstType>::value);
// If the size of {SrcType} and {DstType} matches, we switch to the more
// general and potentially faster {memmove}. Note that this decision is made
// statically at compile time.
// This {memmove} also allows to pass overlapping ranges if the sizes match.
// We probably should not call {CopyChars} with overlapping ranges, but
// unfortunately we sometimes do.
if (sizeof(SrcType) == sizeof(DstType)) {
memmove(dst, src, count * sizeof(SrcType));
return;
}
using SrcTypeUnsigned = typename std::make_unsigned<SrcType>::type;
using DstTypeUnsigned = typename std::make_unsigned<DstType>::type;
#ifdef DEBUG #ifdef DEBUG
// Check for no overlap, otherwise {std::copy_n} cannot be used. // Check for no overlap, otherwise {std::copy_n} cannot be used.
Address src_start = reinterpret_cast<Address>(src); Address src_start = reinterpret_cast<Address>(src);
Address src_end = src_start + count * sizeof(SrcType); Address src_end = src_start + count * sizeof(SrcType);
Address dst_start = reinterpret_cast<Address>(dst); Address dst_start = reinterpret_cast<Address>(dst);
Address dst_end = dst_start + count * sizeof(DstType); Address dst_end = dst_start + count * sizeof(DstType);
DCHECK(src_end < dst_start || dst_end < src_start); DCHECK(src_end <= dst_start || dst_end <= src_start);
#endif #endif
using SrcTypeUnsigned = typename std::make_unsigned<SrcType>::type;
using DstTypeUnsigned = typename std::make_unsigned<DstType>::type;
std::copy_n(reinterpret_cast<const SrcTypeUnsigned*>(src), count, std::copy_n(reinterpret_cast<const SrcTypeUnsigned*>(src), count,
reinterpret_cast<DstTypeUnsigned*>(dst)); reinterpret_cast<DstTypeUnsigned*>(dst));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment