[TurboFan] Enable typed lowering of JSStringConcat to ConsString allocation.

Adds typed lowering of JSStringConcat to ConsString allocation if the
following conditions hold:
 - All concatinations will result in a ConsString of >= ConString::kMinLength
 - No concatinations will result in a empty string in the RHS unless there is
   a sequential string in the LHS.

This also means JSStringConcat needs an eager checkpoint since it can
deopt if throwing a RangeError when the string length protector is valid.

BUG=v8:6243

Change-Id: I01ca79f884df467c10f2c032c72d51b5199c1a3c
Reviewed-on: https://chromium-review.googlesource.com/526636
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46093}

src/compiler/bytecode-graph-builder.cc

@@ -2159,6 +2159,7 @@ void BytecodeGraphBuilder::VisitToPrimitiveToString() {
 }
 
 void BytecodeGraphBuilder::VisitStringConcat() {
+  PrepareEagerCheckpoint();
   interpreter::Register first_reg = bytecode_iterator().GetRegisterOperand(0);
   int operand_count =
       static_cast<int>(bytecode_iterator().GetRegisterCountOperand(1));

src/compiler/js-typed-lowering.cc

@@ -22,6 +22,29 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+namespace {
+
+bool WillCreateConsString(HeapObjectMatcher left, HeapObjectMatcher right) {
+  if (right.HasValue() && right.Value()->IsString()) {
+    Handle<String> right_string = Handle<String>::cast(right.Value());
+    if (right_string->length() >= ConsString::kMinLength) return true;
+  }
+  if (left.HasValue() && left.Value()->IsString()) {
+    Handle<String> left_string = Handle<String>::cast(left.Value());
+    if (left_string->length() >= ConsString::kMinLength) {
+      // The invariant for ConsString requires the left hand side to be
+      // a sequential or external string if the right hand side is the
+      // empty string. Since we don't know anything about the right hand
+      // side here, we must ensure that the left hand side satisfies the
+      // constraints independent of the right hand side.
+      return left_string->IsSeqString() || left_string->IsExternalString();
+    }
+  }
+  return false;
+}
+
+}  // namespace
+
 // A helper class to simplify the process of reducing a single binop node with a
 // JSOperator. This class manages the rewriting of context, control, and effect
 // dependencies during lowering of a binop and contains numerous helper
@@ -106,21 +129,7 @@ class JSBinopReduction final {
         ((lowering_->flags() & JSTypedLowering::kDeoptimizationEnabled) &&
          BinaryOperationHintOf(node_->op()) == BinaryOperationHint::kString)) {
       HeapObjectBinopMatcher m(node_);
-      if (m.right().HasValue() && m.right().Value()->IsString()) {
-        Handle<String> right_string = Handle<String>::cast(m.right().Value());
-        if (right_string->length() >= ConsString::kMinLength) return true;
-      }
-      if (m.left().HasValue() && m.left().Value()->IsString()) {
-        Handle<String> left_string = Handle<String>::cast(m.left().Value());
-        if (left_string->length() >= ConsString::kMinLength) {
-          // The invariant for ConsString requires the left hand side to be
-          // a sequential or external string if the right hand side is the
-          // empty string. Since we don't know anything about the right hand
-          // side here, we must ensure that the left hand side satisfy the
-          // constraints independent of the right hand side.
-          return left_string->IsSeqString() || left_string->IsExternalString();
-        }
-      }
+      return WillCreateConsString(m.left(), m.right());
     }
     return false;
   }
@@ -678,26 +687,9 @@ Reduction JSTypedLowering::ReduceCreateConsString(Node* node) {
     second_type = NodeProperties::GetType(second);
   }
 
-  // Determine the {first} length.
-  HeapObjectBinopMatcher m(node);
-  Node* first_length =
-      (m.left().HasValue() && m.left().Value()->IsString())
-          ? jsgraph()->Constant(
-                Handle<String>::cast(m.left().Value())->length())
-          : effect = graph()->NewNode(
-                simplified()->LoadField(AccessBuilder::ForStringLength()),
-                first, effect, control);
-
-  // Determine the {second} length.
-  Node* second_length =
-      (m.right().HasValue() && m.right().Value()->IsString())
-          ? jsgraph()->Constant(
-                Handle<String>::cast(m.right().Value())->length())
-          : effect = graph()->NewNode(
-                simplified()->LoadField(AccessBuilder::ForStringLength()),
-                second, effect, control);
-
   // Compute the resulting length.
+  Node* first_length = BuildGetStringLength(first, &effect, control);
+  Node* second_length = BuildGetStringLength(second, &effect, control);
   Node* length =
       graph()->NewNode(simplified()->NumberAdd(), first_length, second_length);
 
@@ -716,35 +708,175 @@ Reduction JSTypedLowering::ReduceCreateConsString(Node* node) {
   } else {
     Node* branch =
         graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
-    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-    Node* efalse = effect;
     {
+      Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+      BuildThrowStringRangeError(node, context, frame_state, effect, if_false);
+    }
+    control = graph()->NewNode(common()->IfTrue(), branch);
+  }
+  Node* result = effect =
+      BuildCreateConsString(first, second, length, effect, control);
+  ReplaceWithValue(node, result, effect, control);
+  return Replace(result);
+}
+
+namespace {
+
+// Check if a string concatenation will definitely result in creating a
+// ConsString for all operands, i.e. if the combined length of the first two
+// operands exceeds the ConsString minimum length and we never concatenate the
+// empty string.
+bool ShouldConcatenateAsConsStrings(Node* node) {
+  DCHECK_EQ(IrOpcode::kJSStringConcat, node->opcode());
+  DCHECK_GE(StringConcatParameterOf(node->op()).operand_count(), 3);
+
+  // Check that the concatenation of the first two strings results in a cons
+  // string.
+  HeapObjectMatcher first_matcher(NodeProperties::GetValueInput(node, 0));
+  HeapObjectMatcher second_matcher(NodeProperties::GetValueInput(node, 1));
+  if (!WillCreateConsString(first_matcher, second_matcher)) return false;
+
+  // Now check that all other RHSs of the ConsStrings will be non-empty.
+  int operand_count = StringConcatParameterOf(node->op()).operand_count();
+  for (int i = 2; i < operand_count; ++i) {
+    Node* operand = NodeProperties::GetValueInput(node, i);
+    DCHECK(NodeProperties::GetType(operand)->Is(Type::String()));
+    if (!NodeProperties::GetType(operand)->Is(Type::NonEmptyString())) {
+      return false;
+    }
+  }
+
+  // If all these constraints hold, the result will definitely be a ConsString.
+  return true;
+}
+
+}  // namespace
+
+Reduction JSTypedLowering::ReduceJSStringConcat(Node* node) {
+  if (ShouldConcatenateAsConsStrings(node)) {
+    Node* context = NodeProperties::GetContextInput(node);
+    Node* frame_state = NodeProperties::GetFrameStateInput(node);
+    Node* effect = NodeProperties::GetEffectInput(node);
+    Node* control = NodeProperties::GetControlInput(node);
+    int operand_count = StringConcatParameterOf(node->op()).operand_count();
+
+    // Set up string overflow check dependencies.
+    NodeVector overflow_controls(graph()->zone());
+    NodeVector overflow_effects(graph()->zone());
+    if (isolate()->IsStringLengthOverflowIntact()) {
+      // Add a code dependency on the string length overflow protector.
+      dependencies()->AssumePropertyCell(factory()->string_length_protector());
+    }
+
+    // Get the first operand and its length.
+    Node* current_result = NodeProperties::GetValueInput(node, 0);
+    Node* current_length =
+        BuildGetStringLength(current_result, &effect, control);
+
+    for (int i = 1; i < operand_count; ++i) {
+      bool last_operand = i == operand_count - 1;
+      // Get the next operand and its length.
+      Node* current_operand = NodeProperties::GetValueInput(node, i);
+      HeapObjectMatcher m(current_operand);
+      Node* operand_length =
+          BuildGetStringLength(current_operand, &effect, control);
+
+      // Update the current length and check that it it doesn't overflow.
+      current_length = graph()->NewNode(simplified()->NumberAdd(),
+                                        current_length, operand_length);
+      Node* check = graph()->NewNode(simplified()->NumberLessThanOrEqual(),
+                                     current_length,
+                                     jsgraph()->Constant(String::kMaxLength));
+      if (isolate()->IsStringLengthOverflowIntact()) {
+        // We can just deoptimize if the {check} fails. Besides generating a
+        // shorter code sequence than the version below, this has the additional
+        // benefit of not holding on to the lazy {frame_state} and thus
+        // potentially reduces the number of live ranges and allows for more
+        // truncations.
+        effect =
+            graph()->NewNode(simplified()->CheckIf(), check, effect, control);
+      } else {
+        // Otherwise insert a branch to the runtime call which throws on
+        // overflow.
+        Node* branch = graph()->NewNode(common()->Branch(BranchHint::kTrue),
+                                        check, control);
+        overflow_controls.push_back(
+            graph()->NewNode(common()->IfFalse(), branch));
+        overflow_effects.push_back(effect);
+
+        // Build the string overflow throwing code if we have checked all the
+        // lengths.
+        if (last_operand) {
+          // Merge control and effect of overflow checks.
+          int merge_count = operand_count - 1;
+          DCHECK_EQ(overflow_controls.size(), static_cast<size_t>(merge_count));
+          DCHECK_EQ(overflow_effects.size(), static_cast<size_t>(merge_count));
+
+          Node* if_false =
+              graph()->NewNode(common()->Merge(merge_count), merge_count,
+                               &overflow_controls.front());
+          overflow_effects.push_back(if_false);
+          Node* efalse =
+              graph()->NewNode(common()->EffectPhi(merge_count),
+                               merge_count + 1, &overflow_effects.front());
+
+          // And throw the range error.
+          BuildThrowStringRangeError(node, context, frame_state, efalse,
+                                     if_false);
+        }
+        control = graph()->NewNode(common()->IfTrue(), branch);
+      }
+      current_result = effect = BuildCreateConsString(
+          current_result, current_operand, current_length, effect, control);
+    }
+    ReplaceWithValue(node, current_result, effect, control);
+    return Replace(current_result);
+  }
+  return NoChange();
+}
+
+Node* JSTypedLowering::BuildGetStringLength(Node* value, Node** effect,
+                                            Node* control) {
+  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);
+  return length;
+}
+
+void JSTypedLowering::BuildThrowStringRangeError(Node* node, Node* context,
+                                                 Node* frame_state,
+                                                 Node* effect, Node* control) {
   // Throw a RangeError in case of overflow.
-      Node* vfalse = efalse = if_false = graph()->NewNode(
-          javascript()->CallRuntime(Runtime::kThrowInvalidStringLength),
-          context, frame_state, efalse, if_false);
+  Node* value = effect = control = graph()->NewNode(
+      javascript()->CallRuntime(Runtime::kThrowInvalidStringLength), context,
+      frame_state, effect, control);
 
   // Update potential {IfException} uses of {node} to point to the
   // %ThrowInvalidStringLength runtime call node instead.
   Node* on_exception = nullptr;
   if (NodeProperties::IsExceptionalCall(node, &on_exception)) {
-        NodeProperties::ReplaceControlInput(on_exception, vfalse);
-        NodeProperties::ReplaceEffectInput(on_exception, efalse);
-        if_false = graph()->NewNode(common()->IfSuccess(), vfalse);
+    NodeProperties::ReplaceControlInput(on_exception, value);
+    NodeProperties::ReplaceEffectInput(on_exception, effect);
+    control = graph()->NewNode(common()->IfSuccess(), value);
     Revisit(on_exception);
   }
 
   // The above %ThrowInvalidStringLength runtime call is an unconditional
   // throw, making it impossible to return a successful completion in this
   // case. We simply connect the successful completion to the graph end.
-      if_false = graph()->NewNode(common()->Throw(), efalse, if_false);
+  control = graph()->NewNode(common()->Throw(), effect, control);
   // TODO(bmeurer): This should be on the AdvancedReducer somehow.
-      NodeProperties::MergeControlToEnd(graph(), common(), if_false);
+  NodeProperties::MergeControlToEnd(graph(), common(), control);
   Revisit(graph()->end());
-    }
-    control = graph()->NewNode(common()->IfTrue(), branch);
-  }
+}
 
+Node* JSTypedLowering::BuildCreateConsString(Node* first, Node* second,
+                                             Node* length, Node* effect,
+                                             Node* control) {
   // Figure out the map for the resulting ConsString.
   // TODO(turbofan): We currently just use the cons_string_map here for
   // the sake of simplicity; we could also try to be smarter here and
@@ -773,13 +905,8 @@ Reduction JSTypedLowering::ReduceCreateConsString(Node* node) {
       simplified()->StoreField(AccessBuilder::ForConsStringSecond()), value,
       second, effect, control);
 
-  // Morph the {node} into a {FinishRegion}.
-  ReplaceWithValue(node, node, node, control);
-  node->ReplaceInput(0, value);
-  node->ReplaceInput(1, effect);
-  node->TrimInputCount(2);
-  NodeProperties::ChangeOp(node, common()->FinishRegion());
-  return Changed(node);
+  // Return the {FinishRegion} node.
+  return graph()->NewNode(common()->FinishRegion(), value, effect);
 }
 
 Reduction JSTypedLowering::ReduceSpeculativeNumberComparison(Node* node) {
@@ -2405,6 +2532,8 @@ Reduction JSTypedLowering::Reduce(Node* node) {
       return ReduceJSToString(node);
     case IrOpcode::kJSToPrimitiveToString:
       return ReduceJSToPrimitiveToString(node);
+    case IrOpcode::kJSStringConcat:
+      return ReduceJSStringConcat(node);
     case IrOpcode::kJSToObject:
       return ReduceJSToObject(node);
     case IrOpcode::kJSTypeOf:

src/compiler/js-typed-lowering.h

@@ -69,6 +69,7 @@ class V8_EXPORT_PRIVATE JSTypedLowering final
   Reduction ReduceJSToStringInput(Node* input);
   Reduction ReduceJSToString(Node* node);
   Reduction ReduceJSToPrimitiveToString(Node* node);
+  Reduction ReduceJSStringConcat(Node* node);
   Reduction ReduceJSToObject(Node* node);
   Reduction ReduceJSConvertReceiver(Node* node);
   Reduction ReduceJSConstructForwardVarargs(Node* node);
@@ -94,6 +95,13 @@ 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);
+  void BuildThrowStringRangeError(Node* node, Node* context, Node* frame_state,
+                                  Node* effect, Node* control);
+  Node* BuildCreateConsString(Node* first, Node* second, Node* length,
+                              Node* effect, Node* control);
+
   Factory* factory() const;
   Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }

src/compiler/operator-properties.cc

@@ -108,6 +108,7 @@ bool OperatorProperties::HasFrameStateInput(const Operator* op) {
     case IrOpcode::kJSCallWithSpread:
 
     // Misc operations
+    case IrOpcode::kJSStringConcat:
     case IrOpcode::kJSForInNext:
     case IrOpcode::kJSForInPrepare:
     case IrOpcode::kJSStackCheck:

src/objects-debug.cc

@@ -659,9 +659,7 @@ void String::StringVerify() {
 
 
 void ConsString::ConsStringVerify() {
-  CHECK(this->first()->IsString());
-  CHECK(this->second() == GetHeap()->empty_string() ||
-        this->second()->IsString());
+  CHECK(this->first()->IsString() && this->second()->IsString());
   CHECK(this->length() >= ConsString::kMinLength);
   CHECK(this->length() == this->first()->length() + this->second()->length());
   if (this->IsFlat()) {

test/mjsunit/compiler/string-concat-try-catch.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+var a = "a".repeat(268435440);
+
+(function() {
+  function foo(a) {
+    try {
+      return "longer than ConsString:kMinLength" + a + "0123456789012";
+    } catch (e) {
+      return e;
+    }
+  }
+
+  foo("a");
+  foo("a");
+  // Optimize with string length protector check.
+  %OptimizeFunctionOnNextCall(foo);
+  foo("a");
+  assertInstanceof(foo(a), RangeError);
+  // Optimize without string length protector check.
+  %OptimizeFunctionOnNextCall(foo);
+  foo("a");
+  assertInstanceof(foo(a), RangeError);
+})();