Reland "Remove all custom CopyCharsUnsigned implementations"

This is a reland of 5d8c489000

Original change's description:
> Remove all custom CopyCharsUnsigned implementations
>
> It's unclear whether the custom implementation have any advantage over
> the standard library one's.
> Since we update our toolchain and standard library regularly, it might
> well be the case that the custom implementations are slower by now.
>
> Thus this CL removes all {CopyCharsUnsigned} implementations and
> implements {CopyChars} generically using {std::copy_n}.
>
> Note that this does not touch the {MemMove} and {MemCopy} functions
> yet, as we have seen regressions when trying to remove them before
> (https://crbug.com/v8/8675#c5).
>
> R=leszeks@chromium.org
>
> Bug: v8:9396
> Change-Id: I97a183afebcccd2fbb567bdba02e827331475608
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1800577
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#63808}

Bug: v8:9396
Cq-Include-Trybots: luci.v8.try:v8_linux64_ubsan_rel_ng
Change-Id: I9cd754ebe6b802bb4aabd6d2a448de41da040874
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1807357
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63823}

src/builtins/arm/builtins-arm.cc

@@ -3167,51 +3167,6 @@ void Builtins::Generate_MemCopyUint8Uint8(MacroAssembler* masm) {
   __ Ret();
 }
 
-void Builtins::Generate_MemCopyUint16Uint8(MacroAssembler* masm) {
-  Register dest = r0;
-  Register src = r1;
-  Register chars = r2;
-
-  {
-    UseScratchRegisterScope temps(masm);
-
-    Register temp1 = r3;
-    Register temp2 = temps.Acquire();
-    Register temp3 = lr;
-    Register temp4 = r4;
-    Label loop;
-    Label not_two;
-
-    __ Push(lr, r4);
-    __ bic(temp2, chars, Operand(0x3));
-    __ add(temp2, dest, Operand(temp2, LSL, 1));
-
-    __ bind(&loop);
-    __ ldr(temp1, MemOperand(src, 4, PostIndex));
-    __ uxtb16(temp3, temp1);
-    __ uxtb16(temp4, temp1, 8);
-    __ pkhbt(temp1, temp3, Operand(temp4, LSL, 16));
-    __ str(temp1, MemOperand(dest));
-    __ pkhtb(temp1, temp4, Operand(temp3, ASR, 16));
-    __ str(temp1, MemOperand(dest, 4));
-    __ add(dest, dest, Operand(8));
-    __ cmp(dest, temp2);
-    __ b(&loop, ne);
-
-    __ mov(chars, Operand(chars, LSL, 31), SetCC);  // bit0 => ne, bit1 => cs
-    __ b(&not_two, cc);
-    __ ldrh(temp1, MemOperand(src, 2, PostIndex));
-    __ uxtb(temp3, temp1, 8);
-    __ mov(temp3, Operand(temp3, LSL, 16));
-    __ uxtab(temp3, temp3, temp1);
-    __ str(temp3, MemOperand(dest, 4, PostIndex));
-    __ bind(&not_two);
-    __ ldrb(temp1, MemOperand(src), ne);
-    __ strh(temp1, MemOperand(dest), ne);
-    __ Pop(pc, r4);
-  }
-}
-
 #undef __
 
 }  // namespace internal

src/builtins/builtins-definitions.h

@@ -1113,7 +1113,6 @@ namespace internal {
   TFS(SetProperty, kReceiver, kKey, kValue)                                    \
   TFS(SetPropertyInLiteral, kReceiver, kKey, kValue)                           \
   ASM(MemCopyUint8Uint8, CCall)                                                \
-  ASM(MemCopyUint16Uint8, CCall)                                               \
   ASM(MemMove, CCall)                                                          \
                                                                                \
   /* Trace */                                                                  \

src/builtins/builtins-internal-gen.cc

@@ -933,12 +933,6 @@ void Builtins::Generate_MemCopyUint8Uint8(MacroAssembler* masm) {
 }
 #endif  // !defined(V8_TARGET_ARCH_ARM) && !defined(V8_TARGET_ARCH_MIPS)
 
-#ifndef V8_TARGET_ARCH_ARM
-void Builtins::Generate_MemCopyUint16Uint8(MacroAssembler* masm) {
-  masm->Call(BUILTIN_CODE(masm->isolate(), Illegal), RelocInfo::CODE_TARGET);
-}
-#endif  // V8_TARGET_ARCH_ARM
-
 #ifndef V8_TARGET_ARCH_IA32
 void Builtins::Generate_MemMove(MacroAssembler* masm) {
   masm->Call(BUILTIN_CODE(masm->isolate(), Illegal), RelocInfo::CODE_TARGET);

src/parsing/scanner-character-streams.cc

@@ -265,7 +265,7 @@ class BufferedCharacterStream : public Utf16CharacterStream {
     }
 
     size_t length = Min(kBufferSize, range.length());
-    i::CopyCharsUnsigned(buffer_, range.start, length);
+    i::CopyChars(buffer_, range.start, length);
     buffer_end_ = &buffer_[length];
     return true;
   }

src/utils/memcopy.cc

@@ -25,18 +25,8 @@ V8_EXPORT_PRIVATE void MemMove(void* dest, const void* src, size_t size) {
   (*memmove_function)(dest, src, size);
 }
 #elif V8_OS_POSIX && V8_HOST_ARCH_ARM
-void MemCopyUint16Uint8Wrapper(uint16_t* dest, const uint8_t* src,
-                               size_t chars) {
-  uint16_t* limit = dest + chars;
-  while (dest < limit) {
-    *dest++ = static_cast<uint16_t>(*src++);
-  }
-}
-
 V8_EXPORT_PRIVATE MemCopyUint8Function memcopy_uint8_function =
     &MemCopyUint8Wrapper;
-MemCopyUint16Uint8Function memcopy_uint16_uint8_function =
-    &MemCopyUint16Uint8Wrapper;
 #elif V8_OS_POSIX && V8_HOST_ARCH_MIPS
 V8_EXPORT_PRIVATE MemCopyUint8Function memcopy_uint8_function =
     &MemCopyUint8Wrapper;
@@ -54,9 +44,6 @@ void init_memcopy_functions() {
     EmbeddedData d = EmbeddedData::FromBlob();
     memcopy_uint8_function = reinterpret_cast<MemCopyUint8Function>(
         d.InstructionStartOfBuiltin(Builtins::kMemCopyUint8Uint8));
-    memcopy_uint16_uint8_function =
-        reinterpret_cast<MemCopyUint16Uint8Function>(
-            d.InstructionStartOfBuiltin(Builtins::kMemCopyUint16Uint8));
   }
 #elif V8_OS_POSIX && V8_HOST_ARCH_MIPS
   if (Isolate::CurrentEmbeddedBlobIsBinaryEmbedded()) {

src/utils/memcopy.h

@@ -8,6 +8,7 @@
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
+#include <algorithm>
 
 #include "src/base/logging.h"
 #include "src/base/macros.h"
@@ -55,17 +56,8 @@ V8_EXPORT_PRIVATE V8_INLINE void MemMove(void* dest, const void* src,
   memmove(dest, src, size);
 }
 
-using MemCopyUint16Uint8Function = void (*)(uint16_t* dest, const uint8_t* src,
-                                            size_t size);
-extern MemCopyUint16Uint8Function memcopy_uint16_uint8_function;
-void MemCopyUint16Uint8Wrapper(uint16_t* dest, const uint8_t* src,
-                               size_t chars);
 // For values < 12, the assembler function is slower than the inlined C code.
 const int kMinComplexConvertMemCopy = 12;
-V8_INLINE void MemCopyUint16Uint8(uint16_t* dest, const uint8_t* src,
-                                  size_t size) {
-  (*memcopy_uint16_uint8_function)(dest, src, size);
-}
 #elif defined(V8_HOST_ARCH_MIPS)
 using MemCopyUint8Function = void (*)(uint8_t* dest, const uint8_t* src,
                                       size_t size);
@@ -205,309 +197,44 @@ inline void MemsetPointer(T** dest, U* value, size_t counter) {
                 reinterpret_cast<Address>(value), counter);
 }
 
-template <typename sourcechar, typename sinkchar>
+// Copy from 8bit/16bit chars to 8bit/16bit chars. Values are zero-extended if
-V8_INLINE static void CopyCharsUnsigned(sinkchar* dest, const sourcechar* src,
+// needed. Ranges are not allowed to overlap unless the type sizes match (hence
-                                        size_t chars);
+// {memmove} is used internally).
-#if defined(V8_HOST_ARCH_ARM)
+template <typename SrcType, typename DstType>
-V8_INLINE void CopyCharsUnsigned(uint8_t* dest, const uint8_t* src,
+void CopyChars(DstType* dst, const SrcType* src, size_t count) {
-                                 size_t chars);
+  STATIC_ASSERT(std::is_integral<SrcType>::value);
-V8_INLINE void CopyCharsUnsigned(uint16_t* dest, const uint8_t* src,
+  STATIC_ASSERT(std::is_integral<DstType>::value);
-                                 size_t chars);
+
-V8_INLINE void CopyCharsUnsigned(uint16_t* dest, const uint16_t* src,
+  // This special case for {count == 0} allows to pass {nullptr} as {dst} or
-                                 size_t chars);
+  // {src} in that case.
-#elif defined(V8_HOST_ARCH_MIPS)
+  // TODO(clemensh): Remove this and make {dst} and {src} nonnull.
-V8_INLINE void CopyCharsUnsigned(uint8_t* dest, const uint8_t* src,
+  if (count == 0) return;
-                                 size_t chars);
+
-V8_INLINE void CopyCharsUnsigned(uint16_t* dest, const uint16_t* src,
+  // If the size of {SrcType} and {DstType} matches, we switch to the more
-                                 size_t chars);
+  // general and potentially faster {memmove}. Note that this decision is made
-#elif defined(V8_HOST_ARCH_PPC) || defined(V8_HOST_ARCH_S390)
+  // statically at compile time.
-V8_INLINE void CopyCharsUnsigned(uint8_t* dest, const uint8_t* src,
+  // This {memmove} also allows to pass overlapping ranges if the sizes match.
-                                 size_t chars);
+  // We probably should not call {CopyChars} with overlapping ranges, but
-V8_INLINE void CopyCharsUnsigned(uint16_t* dest, const uint16_t* src,
+  // unfortunately we sometimes do.
-                                 size_t chars);
+  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
+  // Check for no overlap, otherwise {std::copy_n} cannot be used.
+  Address src_start = reinterpret_cast<Address>(src);
+  Address src_end = src_start + count * sizeof(SrcType);
+  Address dst_start = reinterpret_cast<Address>(dst);
+  Address dst_end = dst_start + count * sizeof(DstType);
+  DCHECK(src_end < dst_start || dst_end < src_start);
 #endif
-
+  std::copy_n(reinterpret_cast<const SrcTypeUnsigned*>(src), count,
-// Copy from 8bit/16bit chars to 8bit/16bit chars.
+              reinterpret_cast<DstTypeUnsigned*>(dst));
-template <typename sourcechar, typename sinkchar>
-V8_INLINE void CopyChars(sinkchar* dest, const sourcechar* src, size_t chars);
-
-template <typename sourcechar, typename sinkchar>
-void CopyChars(sinkchar* dest, const sourcechar* src, size_t chars) {
-  DCHECK_LE(sizeof(sourcechar), 2);
-  DCHECK_LE(sizeof(sinkchar), 2);
-  if (sizeof(sinkchar) == 1) {
-    if (sizeof(sourcechar) == 1) {
-      CopyCharsUnsigned(reinterpret_cast<uint8_t*>(dest),
-                        reinterpret_cast<const uint8_t*>(src), chars);
-    } else {
-      CopyCharsUnsigned(reinterpret_cast<uint8_t*>(dest),
-                        reinterpret_cast<const uint16_t*>(src), chars);
-    }
-  } else {
-    if (sizeof(sourcechar) == 1) {
-      CopyCharsUnsigned(reinterpret_cast<uint16_t*>(dest),
-                        reinterpret_cast<const uint8_t*>(src), chars);
-    } else {
-      CopyCharsUnsigned(reinterpret_cast<uint16_t*>(dest),
-                        reinterpret_cast<const uint16_t*>(src), chars);
-    }
-  }
 }
 
-template <typename sourcechar, typename sinkchar>
-void CopyCharsUnsigned(sinkchar* dest, const sourcechar* src, size_t chars) {
-  sinkchar* limit = dest + chars;
-  if ((sizeof(*dest) == sizeof(*src)) &&
-      (chars >= kMinComplexMemCopy / sizeof(*dest))) {
-    MemCopy(dest, src, chars * sizeof(*dest));
-  } else {
-    while (dest < limit) *dest++ = static_cast<sinkchar>(*src++);
-  }
-}
-
-#if defined(V8_HOST_ARCH_ARM)
-void CopyCharsUnsigned(uint8_t* dest, const uint8_t* src, size_t chars) {
-  switch (static_cast<unsigned>(chars)) {
-    case 0:
-      break;
-    case 1:
-      *dest = *src;
-      break;
-    case 2:
-      memcpy(dest, src, 2);
-      break;
-    case 3:
-      memcpy(dest, src, 3);
-      break;
-    case 4:
-      memcpy(dest, src, 4);
-      break;
-    case 5:
-      memcpy(dest, src, 5);
-      break;
-    case 6:
-      memcpy(dest, src, 6);
-      break;
-    case 7:
-      memcpy(dest, src, 7);
-      break;
-    case 8:
-      memcpy(dest, src, 8);
-      break;
-    case 9:
-      memcpy(dest, src, 9);
-      break;
-    case 10:
-      memcpy(dest, src, 10);
-      break;
-    case 11:
-      memcpy(dest, src, 11);
-      break;
-    case 12:
-      memcpy(dest, src, 12);
-      break;
-    case 13:
-      memcpy(dest, src, 13);
-      break;
-    case 14:
-      memcpy(dest, src, 14);
-      break;
-    case 15:
-      memcpy(dest, src, 15);
-      break;
-    default:
-      MemCopy(dest, src, chars);
-      break;
-  }
-}
-
-void CopyCharsUnsigned(uint16_t* dest, const uint8_t* src, size_t chars) {
-  if (chars >= static_cast<size_t>(kMinComplexConvertMemCopy)) {
-    MemCopyUint16Uint8(dest, src, chars);
-  } else {
-    MemCopyUint16Uint8Wrapper(dest, src, chars);
-  }
-}
-
-void CopyCharsUnsigned(uint16_t* dest, const uint16_t* src, size_t chars) {
-  switch (static_cast<unsigned>(chars)) {
-    case 0:
-      break;
-    case 1:
-      *dest = *src;
-      break;
-    case 2:
-      memcpy(dest, src, 4);
-      break;
-    case 3:
-      memcpy(dest, src, 6);
-      break;
-    case 4:
-      memcpy(dest, src, 8);
-      break;
-    case 5:
-      memcpy(dest, src, 10);
-      break;
-    case 6:
-      memcpy(dest, src, 12);
-      break;
-    case 7:
-      memcpy(dest, src, 14);
-      break;
-    default:
-      MemCopy(dest, src, chars * sizeof(*dest));
-      break;
-  }
-}
-
-#elif defined(V8_HOST_ARCH_MIPS)
-void CopyCharsUnsigned(uint8_t* dest, const uint8_t* src, size_t chars) {
-  if (chars < kMinComplexMemCopy) {
-    memcpy(dest, src, chars);
-  } else {
-    MemCopy(dest, src, chars);
-  }
-}
-
-void CopyCharsUnsigned(uint16_t* dest, const uint16_t* src, size_t chars) {
-  if (chars < kMinComplexMemCopy) {
-    memcpy(dest, src, chars * sizeof(*dest));
-  } else {
-    MemCopy(dest, src, chars * sizeof(*dest));
-  }
-}
-#elif defined(V8_HOST_ARCH_PPC) || defined(V8_HOST_ARCH_S390)
-#define CASE(n)           \
-  case n:                 \
-    memcpy(dest, src, n); \
-    break
-void CopyCharsUnsigned(uint8_t* dest, const uint8_t* src, size_t chars) {
-  switch (static_cast<unsigned>(chars)) {
-    case 0:
-      break;
-    case 1:
-      *dest = *src;
-      break;
-      CASE(2);
-      CASE(3);
-      CASE(4);
-      CASE(5);
-      CASE(6);
-      CASE(7);
-      CASE(8);
-      CASE(9);
-      CASE(10);
-      CASE(11);
-      CASE(12);
-      CASE(13);
-      CASE(14);
-      CASE(15);
-      CASE(16);
-      CASE(17);
-      CASE(18);
-      CASE(19);
-      CASE(20);
-      CASE(21);
-      CASE(22);
-      CASE(23);
-      CASE(24);
-      CASE(25);
-      CASE(26);
-      CASE(27);
-      CASE(28);
-      CASE(29);
-      CASE(30);
-      CASE(31);
-      CASE(32);
-      CASE(33);
-      CASE(34);
-      CASE(35);
-      CASE(36);
-      CASE(37);
-      CASE(38);
-      CASE(39);
-      CASE(40);
-      CASE(41);
-      CASE(42);
-      CASE(43);
-      CASE(44);
-      CASE(45);
-      CASE(46);
-      CASE(47);
-      CASE(48);
-      CASE(49);
-      CASE(50);
-      CASE(51);
-      CASE(52);
-      CASE(53);
-      CASE(54);
-      CASE(55);
-      CASE(56);
-      CASE(57);
-      CASE(58);
-      CASE(59);
-      CASE(60);
-      CASE(61);
-      CASE(62);
-      CASE(63);
-      CASE(64);
-    default:
-      memcpy(dest, src, chars);
-      break;
-  }
-}
-#undef CASE
-
-#define CASE(n)               \
-  case n:                     \
-    memcpy(dest, src, n * 2); \
-    break
-void CopyCharsUnsigned(uint16_t* dest, const uint16_t* src, size_t chars) {
-  switch (static_cast<unsigned>(chars)) {
-    case 0:
-      break;
-    case 1:
-      *dest = *src;
-      break;
-      CASE(2);
-      CASE(3);
-      CASE(4);
-      CASE(5);
-      CASE(6);
-      CASE(7);
-      CASE(8);
-      CASE(9);
-      CASE(10);
-      CASE(11);
-      CASE(12);
-      CASE(13);
-      CASE(14);
-      CASE(15);
-      CASE(16);
-      CASE(17);
-      CASE(18);
-      CASE(19);
-      CASE(20);
-      CASE(21);
-      CASE(22);
-      CASE(23);
-      CASE(24);
-      CASE(25);
-      CASE(26);
-      CASE(27);
-      CASE(28);
-      CASE(29);
-      CASE(30);
-      CASE(31);
-      CASE(32);
-    default:
-      memcpy(dest, src, chars * 2);
-      break;
-  }
-}
-#undef CASE
-#endif
-
 }  // namespace internal
 }  // namespace v8