[turbofan] Introduce a dedicated StringLength operator.

Strings are immutable in JavaScript land (contrast with the runtime,
where we can truncate strings that haven't escaped to JavaScript yet),
so the length of a String is immutable. Thus loading the length of a
String is a pure operation and should be expressed as such (i.e. doesn't
depend on control or effect). The StringLength operator does exactly
this and is hooked up to the effect chain in the EffectControlLinearizer.

This will eventually allow us to simplify the optimization of string
concatention and other operations that are a bit cumbersome in TurboFan
currently, and it will also allow us to optimize string operations
across effectful operations, for example combining multiple invocations
to String#slice with the same inputs.

Bug: v8:5269, v8:6936, v8:7109, v8:7137
Change-Id: Iffcccbb0c7fc4cfe1281c10e7af24b40eba4c987
Reviewed-on: https://chromium-review.googlesource.com/799690Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49731}

src/compiler/effect-control-linearizer.cc

@@ -860,6 +860,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
     case IrOpcode::kStringIndexOf:
       result = LowerStringIndexOf(node);
       break;
+    case IrOpcode::kStringLength:
+      result = LowerStringLength(node);
+      break;
     case IrOpcode::kStringToNumber:
       result = LowerStringToNumber(node);
       break;
@@ -2843,6 +2846,12 @@ Node* EffectControlLinearizer::LowerStringIndexOf(Node* node) {
                  position, __ NoContextConstant());
 }
 
+Node* EffectControlLinearizer::LowerStringLength(Node* node) {
+  Node* subject = node->InputAt(0);
+
+  return __ LoadField(AccessBuilder::ForStringLength(), subject);
+}
+
 Node* EffectControlLinearizer::LowerStringComparison(Callable const& callable,
                                                      Node* node) {
   Node* lhs = node->InputAt(0);

src/compiler/effect-control-linearizer.h

@@ -117,6 +117,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
   Node* LowerStringFromCharCode(Node* node);
   Node* LowerStringFromCodePoint(Node* node);
   Node* LowerStringIndexOf(Node* node);
+  Node* LowerStringLength(Node* node);
   Node* LowerStringEqual(Node* node);
   Node* LowerStringLessThan(Node* node);
   Node* LowerStringLessThanOrEqual(Node* node);

src/compiler/js-builtin-reducer.cc

@@ -2391,9 +2391,8 @@ Reduction JSBuiltinReducer::ReduceStringCharAt(Node* node) {
         }
 
         // Determine the {receiver} length.
-        Node* receiver_length = effect = graph()->NewNode(
-            simplified()->LoadField(AccessBuilder::ForStringLength()), receiver,
-            effect, control);
+        Node* receiver_length =
+            graph()->NewNode(simplified()->StringLength(), receiver);
 
         // Check if {index} is less than {receiver} length.
         Node* check = graph()->NewNode(simplified()->NumberLessThan(), index,
@@ -2445,9 +2444,8 @@ Reduction JSBuiltinReducer::ReduceStringCharCodeAt(Node* node) {
         }
 
         // Determine the {receiver} length.
-        Node* receiver_length = effect = graph()->NewNode(
-            simplified()->LoadField(AccessBuilder::ForStringLength()), receiver,
-            effect, control);
+        Node* receiver_length =
+            graph()->NewNode(simplified()->StringLength(), receiver);
 
         // Check if {index} is less than {receiver} length.
         Node* check = graph()->NewNode(simplified()->NumberLessThan(), index,
@@ -2577,9 +2575,7 @@ Reduction JSBuiltinReducer::ReduceStringIteratorNext(Node* node) {
     Node* index = effect = graph()->NewNode(
         simplified()->LoadField(AccessBuilder::ForJSStringIteratorIndex()),
         receiver, effect, control);
-    Node* length = effect = graph()->NewNode(
-        simplified()->LoadField(AccessBuilder::ForStringLength()), string,
-        effect, control);
+    Node* length = graph()->NewNode(simplified()->StringLength(), string);
 
     // branch0: if (index < length)
     Node* check0 =
@@ -2670,9 +2666,8 @@ Reduction JSBuiltinReducer::ReduceStringIteratorNext(Node* node) {
           simplified()->StringFromCodePoint(UnicodeEncoding::UTF16), vtrue0);
 
       // Update iterator.[[NextIndex]]
-      Node* char_length = etrue0 = graph()->NewNode(
-          simplified()->LoadField(AccessBuilder::ForStringLength()), vtrue0,
-          etrue0, if_true0);
+      Node* char_length =
+          graph()->NewNode(simplified()->StringLength(), vtrue0);
       index = graph()->NewNode(simplified()->NumberAdd(), index, char_length);
       etrue0 = graph()->NewNode(
           simplified()->StoreField(AccessBuilder::ForJSStringIteratorIndex()),
@@ -2721,9 +2716,8 @@ Reduction JSBuiltinReducer::ReduceStringSlice(Node* node) {
 
     if (start_type->Is(type_cache_.kSingletonMinusOne) &&
         end_type->Is(Type::Undefined())) {
-      Node* receiver_length = effect = graph()->NewNode(
-          simplified()->LoadField(AccessBuilder::ForStringLength()), receiver,
-          effect, control);
+      Node* receiver_length =
+          graph()->NewNode(simplified()->StringLength(), receiver);
 
       Node* check =
           graph()->NewNode(simplified()->NumberEqual(), receiver_length,

src/compiler/js-native-context-specialization.cc

@@ -1065,9 +1065,7 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
                                          effect, control);
 
     // Determine the {receiver} length.
-    Node* length = effect = graph()->NewNode(
-        simplified()->LoadField(AccessBuilder::ForStringLength()), receiver,
-        effect, control);
+    Node* length = graph()->NewNode(simplified()->StringLength(), receiver);
 
     // Load the single character string from {receiver} or yield undefined
     // if the {index} is out of bounds (depending on the {load_mode}).

src/compiler/js-typed-lowering.cc

@@ -648,8 +648,8 @@ Reduction JSTypedLowering::ReduceCreateConsString(Node* node) {
   }
 
   // Determine the {first} length.
-  Node* first_length = BuildGetStringLength(first, &effect, control);
-  Node* second_length = BuildGetStringLength(second, &effect, control);
+  Node* first_length = BuildGetStringLength(first);
+  Node* second_length = BuildGetStringLength(second);
 
   // Compute the resulting length.
   Node* length =
@@ -708,15 +708,14 @@ Reduction JSTypedLowering::ReduceCreateConsString(Node* node) {
   return Replace(value);
 }
 
-Node* JSTypedLowering::BuildGetStringLength(Node* value, Node** effect,
-                                            Node* control) {
+Node* JSTypedLowering::BuildGetStringLength(Node* value) {
+  // TODO(bmeurer): Get rid of this hack and instead have a way to
+  // express the string length in the types.
   HeapObjectMatcher m(value);
   Node* length =
       (m.HasValue() && m.Value()->IsString())
           ? jsgraph()->Constant(Handle<String>::cast(m.Value())->length())
-          : (*effect) = graph()->NewNode(
-                simplified()->LoadField(AccessBuilder::ForStringLength()),
-                value, *effect, control);
+          : graph()->NewNode(simplified()->StringLength(), value);
   return length;
 }
 
@@ -1118,16 +1117,12 @@ Reduction JSTypedLowering::ReduceJSLoadNamed(Node* node) {
   DCHECK_EQ(IrOpcode::kJSLoadNamed, node->opcode());
   Node* receiver = NodeProperties::GetValueInput(node, 0);
   Type* receiver_type = NodeProperties::GetType(receiver);
-  Node* effect = NodeProperties::GetEffectInput(node);
-  Node* control = NodeProperties::GetControlInput(node);
   Handle<Name> name = NamedAccessOf(node->op()).name();
   // Optimize "length" property of strings.
   if (name.is_identical_to(factory()->length_string()) &&
       receiver_type->Is(Type::String())) {
-    Node* value = effect = graph()->NewNode(
-        simplified()->LoadField(AccessBuilder::ForStringLength()), receiver,
-        effect, control);
-    ReplaceWithValue(node, value, effect);
+    Node* value = graph()->NewNode(simplified()->StringLength(), receiver);
+    ReplaceWithValue(node, value);
     return Replace(value);
   }
   return NoChange();

src/compiler/js-typed-lowering.h

@@ -85,8 +85,8 @@ class V8_EXPORT_PRIVATE JSTypedLowering final
   // Helper for ReduceJSLoadModule and ReduceJSStoreModule.
   Node* BuildGetModuleCell(Node* node);
 
-  // Helpers for ReduceJSCreateConsString and ReduceJSStringConcat.
-  Node* BuildGetStringLength(Node* value, Node** effect, Node* control);
+  // Helpers for ReduceJSCreateConsString.
+  Node* BuildGetStringLength(Node* value);
 
   Factory* factory() const;
   Graph* graph() const;

src/compiler/opcodes.h

@@ -335,6 +335,7 @@
   V(StringFromCharCode)                 \
   V(StringFromCodePoint)                \
   V(StringIndexOf)                      \
+  V(StringLength)                       \
   V(StringToLowerCaseIntl)              \
   V(StringToUpperCaseIntl)              \
   V(CheckBounds)                        \

src/compiler/simplified-lowering.cc

@@ -2415,6 +2415,14 @@ class RepresentationSelector {
         SetOutput(node, MachineRepresentation::kTaggedSigned);
         return;
       }
+      case IrOpcode::kStringLength: {
+        // TODO(bmeurer): The input representation should be TaggedPointer.
+        // Fix this once we have a dedicated StringConcat/JSStringAdd
+        // operator, which marks it's output as TaggedPointer properly.
+        VisitUnop(node, UseInfo::AnyTagged(),
+                  MachineRepresentation::kTaggedSigned);
+        return;
+      }
       case IrOpcode::kStringToLowerCaseIntl:
       case IrOpcode::kStringToUpperCaseIntl: {
         VisitUnop(node, UseInfo::AnyTagged(),

src/compiler/simplified-operator.cc

@@ -590,6 +590,7 @@ DeoptimizeReason DeoptimizeReasonOf(const Operator* op) {
   V(SeqStringCharCodeAt, Operator::kNoProperties, 2, 1)          \
   V(StringFromCharCode, Operator::kNoProperties, 1, 0)           \
   V(StringIndexOf, Operator::kNoProperties, 3, 0)                \
+  V(StringLength, Operator::kNoProperties, 1, 0)                 \
   V(StringToLowerCaseIntl, Operator::kNoProperties, 1, 0)        \
   V(StringToUpperCaseIntl, Operator::kNoProperties, 1, 0)        \
   V(TypeOf, Operator::kNoProperties, 1, 1)                       \

src/compiler/simplified-operator.h

@@ -405,6 +405,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
   const Operator* StringFromCharCode();
   const Operator* StringFromCodePoint(UnicodeEncoding encoding);
   const Operator* StringIndexOf();
+  const Operator* StringLength();
   const Operator* StringToLowerCaseIntl();
   const Operator* StringToUpperCaseIntl();
 

src/compiler/typer.cc

@@ -1978,6 +1978,10 @@ Type* Typer::Visitor::TypeStringFromCodePoint(Node* node) {
 
 Type* Typer::Visitor::TypeStringIndexOf(Node* node) { UNREACHABLE(); }
 
+Type* Typer::Visitor::TypeStringLength(Node* node) {
+  return typer_->cache_.kStringLengthType;
+}
+
 Type* Typer::Visitor::TypeCheckBounds(Node* node) {
   Type* index = Operand(node, 0);
   Type* length = Operand(node, 1);

src/compiler/verifier.cc

@@ -1058,6 +1058,10 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) {
       CheckValueInputIs(node, 2, Type::SignedSmall());
       CheckTypeIs(node, Type::SignedSmall());
       break;
+    case IrOpcode::kStringLength:
+      CheckValueInputIs(node, 0, Type::String());
+      CheckTypeIs(node, TypeCache::Get().kStringLengthType);
+      break;
     case IrOpcode::kStringToLowerCaseIntl:
     case IrOpcode::kStringToUpperCaseIntl:
       CheckValueInputIs(node, 0, Type::String());

test/unittests/compiler/js-typed-lowering-unittest.cc

@@ -382,8 +382,7 @@ TEST_F(JSTypedLoweringTest, JSLoadNamedStringLength) {
       Reduce(graph()->NewNode(javascript()->LoadNamed(name, feedback), receiver,
                               context, EmptyFrameState(), effect, control));
   ASSERT_TRUE(r.Changed());
-  EXPECT_THAT(r.replacement(), IsLoadField(AccessBuilder::ForStringLength(),
-                                           receiver, effect, control));
+  EXPECT_THAT(r.replacement(), IsStringLength(receiver));
 }
 
 

test/unittests/compiler/node-test-utils.cc

@@ -2182,6 +2182,7 @@ IS_UNOP_MATCHER(ObjectIsReceiver)
 IS_UNOP_MATCHER(ObjectIsSmi)
 IS_UNOP_MATCHER(ObjectIsUndetectable)
 IS_UNOP_MATCHER(StringFromCharCode)
+IS_UNOP_MATCHER(StringLength)
 IS_UNOP_MATCHER(Word32Clz)
 IS_UNOP_MATCHER(Word32Ctz)
 IS_UNOP_MATCHER(Word32Popcnt)

test/unittests/compiler/node-test-utils.h

@@ -272,6 +272,7 @@ Matcher<Node*> IsNumberTan(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsNumberTanh(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsNumberTrunc(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsStringFromCharCode(const Matcher<Node*>& value_matcher);
+Matcher<Node*> IsStringLength(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsAllocate(const Matcher<Node*>& size_matcher,
                           const Matcher<Node*>& effect_matcher,
                           const Matcher<Node*>& control_matcher);