[builtins] Refactor StringTrim

This is follow-up of 289d25c1
Improve loop using Subslice, improve common-case check.

Bug: v8:8996
Change-Id: I59ba14b87e1b034fd2d41c92a506e142550363ce
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2527608
Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71666}

src/builtins/convert.tq

@@ -174,6 +174,9 @@ Convert<int32, uint8>(i: uint8): int32 {
 Convert<int32, uint16>(i: uint16): int32 {
   return Signed(Convert<uint32>(i));
 }
+Convert<int32, char16|char8>(i: char16|char8): int32 {
+  return Signed(Convert<uint32>(i));
+}
 Convert<int32, uint31>(i: uint31): int32 {
   return Signed(Convert<uint32>(i));
 }

src/builtins/string-trim.tq

@@ -13,12 +13,17 @@ extern enum TrimMode extends uint31 constexpr 'String::TrimMode' {
 }
 
 @export
-macro IsWhiteSpaceOrLineTerminator(charCode: char16): bool {
+macro IsWhiteSpaceOrLineTerminator(charCode: char16|char8): bool {
   // 0x0020 - SPACE (Intentionally out of order to fast path a commmon case)
   if (charCode == 0x0020) {
     return true;
   }
 
+  // Common Non-whitespace characters from (0x000E, 0x00A0)
+  if (Unsigned(Convert<int32>(charCode) - 0x000E) < 0x0092) {
+    return false;
+  }
+
   // 0x0009 - HORIZONTAL TAB
   if (charCode < 0x0009) {
     return false;
@@ -31,11 +36,6 @@ macro IsWhiteSpaceOrLineTerminator(charCode: char16): bool {
     return true;
   }
 
-  // Common Non-whitespace characters
-  if (charCode < 0x00A0) {
-    return false;
-  }
-
   // 0x00A0 - NO-BREAK SPACE
   if (charCode == 0x00A0) {
     return true;
@@ -92,49 +92,59 @@ macro IsWhiteSpaceOrLineTerminator(charCode: char16): bool {
   return false;
 }
 
-transitioning macro StringTrim(implicit context: Context)(
-    receiver: JSAny, _arguments: Arguments, methodName: constexpr string,
-    variant: constexpr TrimMode): String {
-  const receiverString: String = ToThisString(receiver, methodName);
-  const stringLength: intptr = receiverString.length_intptr;
+transitioning macro StringTrimLoop<T: type>(implicit context: Context)(
+    stringSlice: ConstSlice<T>, startIndex: intptr, endIndex: intptr,
+    increment: intptr): intptr {
+  let index = startIndex;
+  while (true) {
+    if (index == endIndex) {
+      return index;
+    }
+
+    const char: T = *stringSlice.AtIndex(index);
+    if (!IsWhiteSpaceOrLineTerminator(char)) {
+      return index;
+    }
+    index = index + increment;
+  }
+  unreachable;
+}
 
-  const directString = Cast<DirectString>(receiverString)
-      otherwise return runtime::StringTrim(
-      receiverString, SmiTag<TrimMode>(variant));
+transitioning macro StringTrimBody<T: type>(implicit context: Context)(
+    string: String, slice: ConstSlice<T>, variant: constexpr TrimMode): String {
+  const stringLength: intptr = string.length_intptr;
 
   let startIndex: intptr = 0;
   let endIndex: intptr = stringLength - 1;
-
-  // TODO(duongn): It would probably be more efficient to turn StringTrim into a
-  // tempalate for the different string types and specialize the loop for them.
   if (variant == TrimMode::kTrim || variant == TrimMode::kTrimStart) {
-    while (true) {
-      if (startIndex == stringLength) {
-        return EmptyStringConstant();
-      }
-      if (!IsWhiteSpaceOrLineTerminator(
-              StringCharCodeAt(directString, Unsigned(startIndex)))) {
-        break;
-      }
-      startIndex++;
+    startIndex = StringTrimLoop(slice, startIndex, stringLength, 1);
+    if (startIndex == stringLength) {
+      return kEmptyString;
     }
   }
 
   if (variant == TrimMode::kTrim || variant == TrimMode::kTrimEnd) {
-    while (true) {
-      if (endIndex == -1) {
-        return EmptyStringConstant();
-      }
-      if (!IsWhiteSpaceOrLineTerminator(
-              StringCharCodeAt(directString, Unsigned(endIndex)))) {
-        break;
-      }
-      endIndex--;
+    endIndex = StringTrimLoop(slice, endIndex, -1, -1);
+    if (endIndex == -1) {
+      return kEmptyString;
     }
   }
 
-  return SubString(
-      receiverString, Unsigned(startIndex), Unsigned(endIndex + 1));
+  return SubString(string, Unsigned(startIndex), Unsigned(endIndex + 1));
+}
+
+transitioning macro StringTrim(implicit context: Context)(
+    receiver: JSAny, _arguments: Arguments, methodName: constexpr string,
+    variant: constexpr TrimMode): String {
+  const receiverString: String = ToThisString(receiver, methodName);
+
+  try {
+    GetStringData(receiverString) otherwise OneByte, TwoByte;
+  } label OneByte(slice: ConstSlice<char8>) {
+    return StringTrimBody(receiverString, slice, variant);
+  } label TwoByte(slice: ConstSlice<char16>) {
+    return StringTrimBody(receiverString, slice, variant);
+  }
 }
 
 // ES6 #sec-string.prototype.trim
@@ -161,8 +171,3 @@ StringPrototypeTrimEnd(
   return StringTrim(receiver, arguments, methodName, TrimMode::kTrimEnd);
 }
 }
-
-namespace runtime {
-extern runtime StringTrim(implicit context: Context)(
-    String, SmiTagged<string::TrimMode>): String;
-}

src/debug/debug-evaluate.cc

@@ -335,7 +335,6 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
   V(StringReplaceOneCharWithString)           \
   V(StringSubstring)                          \
   V(StringToNumber)                           \
-  V(StringTrim)                               \
   /* BigInts */                               \
   V(BigIntEqualToBigInt)                      \
   V(BigIntToBoolean)                          \

src/objects/string.cc

@@ -411,32 +411,6 @@ void String::PrintUC16(StringStream* accumulator, int start, int end) {
   }
 }
 
-// static
-Handle<String> String::Trim(Isolate* isolate, Handle<String> string,
-                            TrimMode mode) {
-  string = String::Flatten(isolate, string);
-  int const length = string->length();
-
-  // Perform left trimming if requested.
-  int left = 0;
-  if (mode == kTrim || mode == kTrimStart) {
-    while (left < length && IsWhiteSpaceOrLineTerminator(string->Get(left))) {
-      left++;
-    }
-  }
-
-  // Perform right trimming if requested.
-  int right = length;
-  if (mode == kTrim || mode == kTrimEnd) {
-    while (right > left &&
-           IsWhiteSpaceOrLineTerminator(string->Get(right - 1))) {
-      right--;
-    }
-  }
-
-  return isolate->factory()->NewSubString(string, left, right);
-}
-
 int32_t String::ToArrayIndex(Address addr) {
   DisallowGarbageCollection no_gc;
   String key(addr);

src/objects/string.h

@@ -385,8 +385,6 @@ class String : public TorqueGeneratedString<String, Name> {
 
   // Trimming.
   enum TrimMode { kTrim, kTrimStart, kTrimEnd };
-  static Handle<String> Trim(Isolate* isolate, Handle<String> string,
-                             TrimMode mode);
 
   V8_EXPORT_PRIVATE void PrintOn(FILE* out);
 

src/runtime/runtime-strings.cc

@@ -139,15 +139,6 @@ RUNTIME_FUNCTION(Runtime_StringReplaceOneCharWithString) {
   return isolate->StackOverflow();
 }
 
-RUNTIME_FUNCTION(Runtime_StringTrim) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(2, args.length());
-  Handle<String> string = args.at<String>(0);
-  CONVERT_SMI_ARG_CHECKED(mode, 1);
-  String::TrimMode trim_mode = static_cast<String::TrimMode>(mode);
-  return *String::Trim(isolate, string, trim_mode);
-}
-
 // ES6 #sec-string.prototype.includes
 // String.prototype.includes(searchString [, position])
 RUNTIME_FUNCTION(Runtime_StringIncludes) {

src/runtime/runtime.h

@@ -440,8 +440,7 @@ namespace internal {
   F(StringMaxLength, 0, 1)                \
   F(StringReplaceOneCharWithString, 3, 1) \
   F(StringSubstring, 3, 1)                \
-  F(StringToArray, 2, 1)                  \
-  F(StringTrim, 2, 1)
+  F(StringToArray, 2, 1)
 
 #define FOR_EACH_INTRINSIC_SYMBOL(F, I)    \
   F(CreatePrivateNameSymbol, 1, 1)         \