From f2e987d6e71f94cf04004e9958d725dac1e05183 Mon Sep 17 00:00:00 2001
From: Leszek Swirski <leszeks@chromium.org>
Date: Tue, 14 Sep 2021 10:32:39 +0200
Subject: [PATCH] [string] Make WriteToFlat take 'length' instead of 'end'

CopyChars takes a count parameter, not an end parameter, so we can save
some subtractions by passing in the count to WriteToFlat. Most of the
time the start,end arguments into WriteToFlat are 0,length anyway, so
this doesn't change a lot of places.

Change-Id: I9587c7afce529218a16b728c0477b87569df8e21
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3157947
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76813}
---
 src/api/api.cc                     |  9 ++--
 src/codegen/code-stub-assembler.cc | 16 +++----
 src/codegen/code-stub-assembler.h  |  4 +-
 src/codegen/external-reference.cc  | 16 +++----
 src/heap/factory.cc                |  4 +-
 src/objects/string.cc              | 76 +++++++++++++++---------------
 src/runtime/runtime-regexp.cc      |  8 ++--
 src/strings/string-builder.cc      |  2 +-
 8 files changed, 69 insertions(+), 66 deletions(-)

diff --git a/src/api/api.cc b/src/api/api.cc
index 9102f6b3c5d..2f6cfd4374d 100644
--- a/src/api/api.cc
+++ b/src/api/api.cc
@@ -5626,12 +5626,13 @@ static inline int WriteHelper(i::Isolate* isolate, const String* string,
   int end = start + length;
   if ((length == -1) || (length > str->length() - start)) end = str->length();
   if (end < 0) return 0;
-  if (start < end) i::String::WriteToFlat(*str, buffer, start, end);
+  int write_length = end - start;
+  if (start < end) i::String::WriteToFlat(*str, buffer, start, write_length);
   if (!(options & String::NO_NULL_TERMINATION) &&
-      (length == -1 || end - start < length)) {
-    buffer[end - start] = '\0';
+      (length == -1 || write_length < length)) {
+    buffer[write_length] = '\0';
   }
-  return end - start;
+  return write_length;
 }
 
 int String::WriteOneByte(Isolate* isolate, uint8_t* buffer, int start,
diff --git a/src/codegen/code-stub-assembler.cc b/src/codegen/code-stub-assembler.cc
index 92686eff124..4f2ced4e862 100644
--- a/src/codegen/code-stub-assembler.cc
+++ b/src/codegen/code-stub-assembler.cc
@@ -8015,28 +8015,28 @@ void CodeStubAssembler::TryToName(TNode<Object> key, Label* if_keyisindex,
 
 void CodeStubAssembler::StringWriteToFlatOneByte(TNode<String> source,
                                                  TNode<RawPtrT> sink,
-                                                 TNode<Int32T> from,
-                                                 TNode<Int32T> to) {
+                                                 TNode<Int32T> start,
+                                                 TNode<Int32T> length) {
   TNode<ExternalReference> function =
       ExternalConstant(ExternalReference::string_write_to_flat_one_byte());
   CallCFunction(function, base::nullopt,
                 std::make_pair(MachineType::AnyTagged(), source),
                 std::make_pair(MachineType::Pointer(), sink),
-                std::make_pair(MachineType::Int32(), from),
-                std::make_pair(MachineType::Int32(), to));
+                std::make_pair(MachineType::Int32(), start),
+                std::make_pair(MachineType::Int32(), length));
 }
 
 void CodeStubAssembler::StringWriteToFlatTwoByte(TNode<String> source,
                                                  TNode<RawPtrT> sink,
-                                                 TNode<Int32T> from,
-                                                 TNode<Int32T> to) {
+                                                 TNode<Int32T> start,
+                                                 TNode<Int32T> length) {
   TNode<ExternalReference> function =
       ExternalConstant(ExternalReference::string_write_to_flat_two_byte());
   CallCFunction(function, base::nullopt,
                 std::make_pair(MachineType::AnyTagged(), source),
                 std::make_pair(MachineType::Pointer(), sink),
-                std::make_pair(MachineType::Int32(), from),
-                std::make_pair(MachineType::Int32(), to));
+                std::make_pair(MachineType::Int32(), start),
+                std::make_pair(MachineType::Int32(), length));
 }
 
 TNode<RawPtr<Uint8T>> CodeStubAssembler::ExternalOneByteStringGetChars(
diff --git a/src/codegen/code-stub-assembler.h b/src/codegen/code-stub-assembler.h
index f869ac687f4..b73bf46ac91 100644
--- a/src/codegen/code-stub-assembler.h
+++ b/src/codegen/code-stub-assembler.h
@@ -2881,9 +2881,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
 
   // Call non-allocating runtime String::WriteToFlat using fast C-calls.
   void StringWriteToFlatOneByte(TNode<String> source, TNode<RawPtrT> sink,
-                                TNode<Int32T> from, TNode<Int32T> to);
+                                TNode<Int32T> start, TNode<Int32T> length);
   void StringWriteToFlatTwoByte(TNode<String> source, TNode<RawPtrT> sink,
-                                TNode<Int32T> from, TNode<Int32T> to);
+                                TNode<Int32T> start, TNode<Int32T> length);
 
   // Calls External{One,Two}ByteString::GetChars with a fast C-call.
   TNode<RawPtr<Uint8T>> ExternalOneByteStringGetChars(
diff --git a/src/codegen/external-reference.cc b/src/codegen/external-reference.cc
index 0c04e84a68b..948de10bc4e 100644
--- a/src/codegen/external-reference.cc
+++ b/src/codegen/external-reference.cc
@@ -882,16 +882,16 @@ ExternalReference ExternalReference::search_string_raw_two_two() {
 
 namespace {
 
-void StringWriteToFlatOneByte(Address source, uint8_t* sink, int32_t from,
-                              int32_t to) {
-  return String::WriteToFlat<uint8_t>(String::cast(Object(source)), sink, from,
-                                      to);
+void StringWriteToFlatOneByte(Address source, uint8_t* sink, int32_t start,
+                              int32_t length) {
+  return String::WriteToFlat<uint8_t>(String::cast(Object(source)), sink, start,
+                                      length);
 }
 
-void StringWriteToFlatTwoByte(Address source, uint16_t* sink, int32_t from,
-                              int32_t to) {
-  return String::WriteToFlat<uint16_t>(String::cast(Object(source)), sink, from,
-                                       to);
+void StringWriteToFlatTwoByte(Address source, uint16_t* sink, int32_t start,
+                              int32_t length) {
+  return String::WriteToFlat<uint16_t>(String::cast(Object(source)), sink,
+                                       start, length);
 }
 
 const uint8_t* ExternalOneByteStringGetChars(Address string) {
diff --git a/src/heap/factory.cc b/src/heap/factory.cc
index e995a498976..c9c02080581 100644
--- a/src/heap/factory.cc
+++ b/src/heap/factory.cc
@@ -1021,14 +1021,14 @@ Handle<String> Factory::NewProperSubString(Handle<String> str, int begin,
           NewRawOneByteString(length).ToHandleChecked();
       DisallowGarbageCollection no_gc;
       uint8_t* dest = result->GetChars(no_gc);
-      String::WriteToFlat(*str, dest, begin, end);
+      String::WriteToFlat(*str, dest, begin, length);
       return result;
     } else {
       Handle<SeqTwoByteString> result =
           NewRawTwoByteString(length).ToHandleChecked();
       DisallowGarbageCollection no_gc;
       base::uc16* dest = result->GetChars(no_gc);
-      String::WriteToFlat(*str, dest, begin, end);
+      String::WriteToFlat(*str, dest, begin, length);
       return result;
     }
   }
diff --git a/src/objects/string.cc b/src/objects/string.cc
index f2f9bf05200..cdbc9afda8d 100644
--- a/src/objects/string.cc
+++ b/src/objects/string.cc
@@ -645,88 +645,91 @@ std::unique_ptr<char[]> String::ToCString(AllowNullsFlag allow_nulls,
 
 // static
 template <typename sinkchar>
-void String::WriteToFlat(String source, sinkchar* sink, int from, int to) {
+void String::WriteToFlat(String source, sinkchar* sink, int start, int length) {
   DCHECK(!SharedStringAccessGuardIfNeeded::IsNeeded(source));
-  return WriteToFlat(source, sink, from, to, GetPtrComprCageBase(source),
+  return WriteToFlat(source, sink, start, length, GetPtrComprCageBase(source),
                      SharedStringAccessGuardIfNeeded::NotNeeded());
 }
 
 // static
 template <typename sinkchar>
-void String::WriteToFlat(String source, sinkchar* sink, int from, int to,
+void String::WriteToFlat(String source, sinkchar* sink, int start, int length,
                          PtrComprCageBase cage_base,
                          const SharedStringAccessGuardIfNeeded& access_guard) {
   DisallowGarbageCollection no_gc;
-  if (from == to) return;
+  if (length == 0) return;
   while (true) {
-    DCHECK_LT(from, to);
-    DCHECK_LE(0, from);
-    DCHECK_LE(to, source.length());
+    DCHECK_LT(0, length);
+    DCHECK_LE(0, start);
+    DCHECK_LE(length, source.length());
     switch (StringShape(source, cage_base).full_representation_tag()) {
       case kOneByteStringTag | kExternalStringTag:
-        CopyChars(sink, ExternalOneByteString::cast(source).GetChars() + from,
-                  to - from);
+        CopyChars(sink, ExternalOneByteString::cast(source).GetChars() + start,
+                  length);
         return;
       case kTwoByteStringTag | kExternalStringTag:
-        CopyChars(sink, ExternalTwoByteString::cast(source).GetChars() + from,
-                  to - from);
+        CopyChars(sink, ExternalTwoByteString::cast(source).GetChars() + start,
+                  length);
         return;
       case kOneByteStringTag | kSeqStringTag:
-        CopyChars(
-            sink,
-            SeqOneByteString::cast(source).GetChars(no_gc, access_guard) + from,
-            to - from);
+        CopyChars(sink,
+                  SeqOneByteString::cast(source).GetChars(no_gc, access_guard) +
+                      start,
+                  length);
         return;
       case kTwoByteStringTag | kSeqStringTag:
-        CopyChars(
-            sink,
-            SeqTwoByteString::cast(source).GetChars(no_gc, access_guard) + from,
-            to - from);
+        CopyChars(sink,
+                  SeqTwoByteString::cast(source).GetChars(no_gc, access_guard) +
+                      start,
+                  length);
         return;
       case kOneByteStringTag | kConsStringTag:
       case kTwoByteStringTag | kConsStringTag: {
         ConsString cons_string = ConsString::cast(source);
         String first = cons_string.first(cage_base);
         int boundary = first.length();
-        if (to - boundary >= boundary - from) {
+        int first_length = boundary - start;
+        int second_length = start + length - boundary;
+        if (second_length >= first_length) {
           // Right hand side is longer.  Recurse over left.
-          if (from < boundary) {
-            WriteToFlat(first, sink, from, boundary, cage_base, access_guard);
-            if (from == 0 && cons_string.second(cage_base) == first) {
+          if (first_length > 0) {
+            WriteToFlat(first, sink, start, first_length, cage_base,
+                        access_guard);
+            if (start == 0 && cons_string.second(cage_base) == first) {
               CopyChars(sink + boundary, sink, boundary);
               return;
             }
-            sink += boundary - from;
-            from = 0;
+            sink += boundary - start;
+            start = 0;
+            length -= first_length;
           } else {
-            from -= boundary;
+            start -= boundary;
           }
-          to -= boundary;
           source = cons_string.second(cage_base);
         } else {
           // Left hand side is longer.  Recurse over right.
-          if (to > boundary) {
+          if (second_length > 0) {
             String second = cons_string.second(cage_base);
             // When repeatedly appending to a string, we get a cons string that
             // is unbalanced to the left, a list, essentially.  We inline the
             // common case of sequential one-byte right child.
-            if (to - boundary == 1) {
-              sink[boundary - from] =
+            if (second_length == 1) {
+              sink[boundary - start] =
                   static_cast<sinkchar>(second.Get(0, cage_base, access_guard));
             } else if (second.IsSeqOneByteString(cage_base)) {
               CopyChars(
-                  sink + boundary - from,
+                  sink + boundary - start,
                   SeqOneByteString::cast(second).GetChars(no_gc, access_guard),
-                  to - boundary);
+                  second_length);
             } else {
-              WriteToFlat(second, sink + boundary - from, 0, to - boundary,
+              WriteToFlat(second, sink + boundary - start, 0, second_length,
                           cage_base, access_guard);
             }
-            to = boundary;
+            length -= second_length;
           }
           source = first;
         }
-        if (from == to) return;
+        if (length == 0) return;
         continue;
       }
       case kOneByteStringTag | kSlicedStringTag:
@@ -734,8 +737,7 @@ void String::WriteToFlat(String source, sinkchar* sink, int from, int to,
         SlicedString slice = SlicedString::cast(source);
         unsigned offset = slice.offset();
         source = slice.parent(cage_base);
-        from += offset;
-        to += offset;
+        start += offset;
         continue;
       }
       case kOneByteStringTag | kThinStringTag:
diff --git a/src/runtime/runtime-regexp.cc b/src/runtime/runtime-regexp.cc
index c52449a642b..94175acd9a5 100644
--- a/src/runtime/runtime-regexp.cc
+++ b/src/runtime/runtime-regexp.cc
@@ -595,7 +595,7 @@ V8_WARN_UNUSED_RESULT static Object StringReplaceGlobalAtomRegExpWithString(
     // Copy non-matched subject content.
     if (subject_pos < index) {
       String::WriteToFlat(*subject, result->GetChars(no_gc) + result_pos,
-                          subject_pos, index);
+                          subject_pos, index - subject_pos);
       result_pos += index - subject_pos;
     }
 
@@ -611,7 +611,7 @@ V8_WARN_UNUSED_RESULT static Object StringReplaceGlobalAtomRegExpWithString(
   // Add remaining subject content at the end.
   if (subject_pos < subject_len) {
     String::WriteToFlat(*subject, result->GetChars(no_gc) + result_pos,
-                        subject_pos, subject_len);
+                        subject_pos, subject_len - subject_pos);
   }
 
   int32_t match_indices[] = {indices->back(), indices->back() + pattern_len};
@@ -753,7 +753,7 @@ V8_WARN_UNUSED_RESULT static Object StringReplaceGlobalRegExpWithEmptyString(
     if (prev < start) {
       // Add substring subject[prev;start] to answer string.
       String::WriteToFlat(*subject, answer->GetChars(no_gc) + position, prev,
-                          start);
+                          start - prev);
       position += start - prev;
     }
     prev = end;
@@ -769,7 +769,7 @@ V8_WARN_UNUSED_RESULT static Object StringReplaceGlobalRegExpWithEmptyString(
   if (prev < subject_length) {
     // Add substring subject[prev;length] to answer string.
     String::WriteToFlat(*subject, answer->GetChars(no_gc) + position, prev,
-                        subject_length);
+                        subject_length - prev);
     position += subject_length - prev;
   }
 
diff --git a/src/strings/string-builder.cc b/src/strings/string-builder.cc
index 71534d635fd..7135d556bc3 100644
--- a/src/strings/string-builder.cc
+++ b/src/strings/string-builder.cc
@@ -34,7 +34,7 @@ void StringBuilderConcatHelper(String special, sinkchar* sink,
         pos = Smi::ToInt(obj);
         len = -encoded_slice;
       }
-      String::WriteToFlat(special, sink + position, pos, pos + len);
+      String::WriteToFlat(special, sink + position, pos, len);
       position += len;
     } else {
       String string = String::cast(element);
-- 
2.18.1