[turbofan] Introduce a pure StringConcat operator.

This replaces the previous CheckStringAdd operator which deopts in case
the combined length overflows with a dedicated pure StringConcat operator.
This operator is similar to NewConsString in that it takes the resulting
length plus the two input strings. The operator relies on the length
being checked explicitly by the surrounding code instead of baking the
check into the operator itself. This way TurboFan can eliminate
redundant/unnecessary StringConcat operations, since they are pure now.

This also unifies the treatment of string addition in JSTypedLowering,
and generalizes the StringLength constant-folding to apply to more cases
not just the JSAdd cases inside JSTypedLowering.

Bug: v8:7902, v8:8015
Change-Id: I987ec39815a9464fd5fd9c4f7b26b709f94f2b3f
Reviewed-on: https://chromium-review.googlesource.com/1213205Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55725}

src/compiler/effect-control-linearizer.cc

@@ -694,9 +694,6 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
     case IrOpcode::kCheckIf:
       LowerCheckIf(node, frame_state);
       break;
-    case IrOpcode::kCheckStringAdd:
-      result = LowerCheckStringAdd(node, frame_state);
-      break;
     case IrOpcode::kCheckedInt32Add:
       result = LowerCheckedInt32Add(node, frame_state);
       break;
@@ -836,6 +833,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
     case IrOpcode::kDeadValue:
       result = LowerDeadValue(node);
       break;
+    case IrOpcode::kStringConcat:
+      result = LowerStringConcat(node);
+      break;
     case IrOpcode::kStringFromSingleCharCode:
       result = LowerStringFromSingleCharCode(node);
       break;
@@ -1547,18 +1547,9 @@ void EffectControlLinearizer::LowerCheckIf(Node* node, Node* frame_state) {
   __ DeoptimizeIfNot(p.reason(), p.feedback(), value, frame_state);
 }
 
-Node* EffectControlLinearizer::LowerCheckStringAdd(Node* node,
-                                                   Node* frame_state) {
-  Node* lhs = node->InputAt(0);
-  Node* rhs = node->InputAt(1);
-
-  Node* lhs_length = __ LoadField(AccessBuilder::ForStringLength(), lhs);
-  Node* rhs_length = __ LoadField(AccessBuilder::ForStringLength(), rhs);
-  Node* check = __ IntLessThan(__ IntAdd(lhs_length, rhs_length),
-                               __ SmiConstant(String::kMaxLength));
-
-  __ DeoptimizeIfNot(DeoptimizeReason::kOverflow, VectorSlotPair(), check,
-                     frame_state);
+Node* EffectControlLinearizer::LowerStringConcat(Node* node) {
+  Node* lhs = node->InputAt(1);
+  Node* rhs = node->InputAt(2);
 
   Callable const callable =
       CodeFactory::StringAdd(isolate(), STRING_ADD_CHECK_NONE, NOT_TENURED);

src/compiler/effect-control-linearizer.h

@@ -67,7 +67,6 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
   Node* LowerCheckString(Node* node, Node* frame_state);
   Node* LowerCheckSymbol(Node* node, Node* frame_state);
   void LowerCheckIf(Node* node, Node* frame_state);
-  Node* LowerCheckStringAdd(Node* node, Node* frame_state);
   Node* LowerCheckedInt32Add(Node* node, Node* frame_state);
   Node* LowerCheckedInt32Sub(Node* node, Node* frame_state);
   Node* LowerCheckedInt32Div(Node* node, Node* frame_state);
@@ -124,6 +123,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
   Node* LowerArrayBufferWasNeutered(Node* node);
   Node* LowerSameValue(Node* node);
   Node* LowerDeadValue(Node* node);
+  Node* LowerStringConcat(Node* node);
   Node* LowerStringToNumber(Node* node);
   Node* LowerStringCharCodeAt(Node* node);
   Node* LowerStringCodePointAt(Node* node, UnicodeEncoding encoding);

src/compiler/js-call-reducer.cc

@@ -5452,6 +5452,7 @@ Reduction JSCallReducer::ReduceStringPrototypeConcat(
   if (p.speculation_mode() == SpeculationMode::kDisallowSpeculation) {
     return NoChange();
   }
+
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
   Node* receiver = effect =
@@ -5463,16 +5464,21 @@ Reduction JSCallReducer::ReduceStringPrototypeConcat(
     return Replace(receiver);
   }
 
-  if (!isolate()->IsStringLengthOverflowIntact()) {
-    return NoChange();
-  }
-
   Node* argument = effect =
       graph()->NewNode(simplified()->CheckString(p.feedback()),
                        NodeProperties::GetValueInput(node, 2), effect, control);
-
-  Node* value = effect = graph()->NewNode(simplified()->CheckStringAdd(),
-                                          receiver, argument, effect, control);
+  Node* receiver_length =
+      graph()->NewNode(simplified()->StringLength(), receiver);
+  Node* argument_length =
+      graph()->NewNode(simplified()->StringLength(), argument);
+  Node* length = graph()->NewNode(simplified()->NumberAdd(), receiver_length,
+                                  argument_length);
+  length = effect = graph()->NewNode(
+      simplified()->CheckBounds(p.feedback()), length,
+      jsgraph()->Constant(String::kMaxLength + 1), effect, control);
+
+  Node* value = graph()->NewNode(simplified()->StringConcat(), length, receiver,
+                                 argument);
 
   ReplaceWithValue(node, value, effect, control);
   return Replace(value);

src/compiler/js-typed-lowering.h

@@ -82,7 +82,6 @@ class V8_EXPORT_PRIVATE JSTypedLowering final
   Reduction ReduceNumberBinop(Node* node);
   Reduction ReduceInt32Binop(Node* node);
   Reduction ReduceUI32Shift(Node* node, Signedness signedness);
-  Reduction ReduceCreateConsString(Node* node);
   Reduction ReduceSpeculativeNumberAdd(Node* node);
   Reduction ReduceSpeculativeNumberMultiply(Node* node);
   Reduction ReduceSpeculativeNumberBinop(Node* node);
@@ -93,9 +92,6 @@ class V8_EXPORT_PRIVATE JSTypedLowering final
   // Helper for ReduceJSLoadModule and ReduceJSStoreModule.
   Node* BuildGetModuleCell(Node* node);
 
-  // Helpers for ReduceJSCreateConsString.
-  Node* BuildGetStringLength(Node* value);
-
   Factory* factory() const;
   Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }

src/compiler/opcodes.h

@@ -349,6 +349,7 @@
   V(PlainPrimitiveToWord32)             \
   V(PlainPrimitiveToFloat64)            \
   V(BooleanNot)                         \
+  V(StringConcat)                       \
   V(StringToNumber)                     \
   V(StringCharCodeAt)                   \
   V(StringCodePointAt)                  \
@@ -373,7 +374,6 @@
   V(CheckNotTaggedHole)                 \
   V(CheckEqualsInternalizedString)      \
   V(CheckEqualsSymbol)                  \
-  V(CheckStringAdd)                     \
   V(CompareMaps)                        \
   V(ConvertReceiver)                    \
   V(ConvertTaggedHoleToUndefined)       \

src/compiler/redundancy-elimination.cc

@@ -32,7 +32,6 @@ Reduction RedundancyElimination::Reduce(Node* node) {
     case IrOpcode::kCheckSmi:
     case IrOpcode::kCheckString:
     case IrOpcode::kCheckSymbol:
-    case IrOpcode::kCheckStringAdd:
     case IrOpcode::kCheckedFloat64ToInt32:
     case IrOpcode::kCheckedInt32Add:
     case IrOpcode::kCheckedInt32Div:

src/compiler/simplified-lowering.cc

@@ -2344,6 +2344,18 @@ class RepresentationSelector {
         SetOutput(node, MachineRepresentation::kTaggedPointer);
         return;
       }
+      case IrOpcode::kStringConcat: {
+        // TODO(turbofan): We currently depend on having this first length input
+        // to make sure that the overflow check is properly scheduled before the
+        // actual string concatenation. We should also use the length to pass it
+        // to the builtin or decide in optimized code how to construct the
+        // resulting string (i.e. cons string or sequential string).
+        ProcessInput(node, 0, UseInfo::TaggedSigned());  // length
+        ProcessInput(node, 1, UseInfo::AnyTagged());     // first
+        ProcessInput(node, 2, UseInfo::AnyTagged());     // second
+        SetOutput(node, MachineRepresentation::kTaggedPointer);
+        return;
+      }
       case IrOpcode::kStringEqual:
       case IrOpcode::kStringLessThan:
       case IrOpcode::kStringLessThanOrEqual: {
@@ -3141,7 +3153,6 @@ class RepresentationSelector {
       case IrOpcode::kArgumentsLengthState:
       case IrOpcode::kUnreachable:
       case IrOpcode::kRuntimeAbort:
-      case IrOpcode::kCheckStringAdd:
 // All JavaScript operators except JSToNumber have uniform handling.
 #define OPCODE_CASE(name) case IrOpcode::k##name:
         JS_SIMPLE_BINOP_LIST(OPCODE_CASE)

src/compiler/simplified-operator.cc

@@ -709,6 +709,7 @@ bool operator==(CheckMinusZeroParameters const& lhs,
   V(NumberToUint32, Operator::kNoProperties, 1, 0)               \
   V(NumberToUint8Clamped, Operator::kNoProperties, 1, 0)         \
   V(NumberSilenceNaN, Operator::kNoProperties, 1, 0)             \
+  V(StringConcat, Operator::kNoProperties, 3, 0)                 \
   V(StringToNumber, Operator::kNoProperties, 1, 0)               \
   V(StringFromSingleCharCode, Operator::kNoProperties, 1, 0)     \
   V(StringIndexOf, Operator::kNoProperties, 3, 0)                \
@@ -790,8 +791,7 @@ bool operator==(CheckMinusZeroParameters const& lhs,
   V(CheckedInt32Mod, 2, 1)               \
   V(CheckedInt32Sub, 2, 1)               \
   V(CheckedUint32Div, 2, 1)              \
-  V(CheckedUint32Mod, 2, 1)              \
-  V(CheckStringAdd, 2, 1)
+  V(CheckedUint32Mod, 2, 1)
 
 #define CHECKED_WITH_FEEDBACK_OP_LIST(V) \
   V(CheckBounds, 2, 1)                   \

src/compiler/simplified-operator.h

@@ -616,6 +616,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
 
   const Operator* ToBoolean();
 
+  const Operator* StringConcat();
   const Operator* StringEqual();
   const Operator* StringLessThan();
   const Operator* StringLessThanOrEqual();
@@ -678,8 +679,6 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
   const Operator* CheckString(const VectorSlotPair& feedback);
   const Operator* CheckSymbol();
 
-  const Operator* CheckStringAdd();
-
   const Operator* CheckedFloat64ToInt32(CheckForMinusZeroMode,
                                         const VectorSlotPair& feedback);
   const Operator* CheckedInt32Add();

src/compiler/typed-optimization.cc

@@ -71,6 +71,8 @@ Reduction TypedOptimization::Reduce(Node* node) {
     case IrOpcode::kStringLessThan:
     case IrOpcode::kStringLessThanOrEqual:
       return ReduceStringComparison(node);
+    case IrOpcode::kStringLength:
+      return ReduceStringLength(node);
     case IrOpcode::kSameValue:
       return ReduceSameValue(node);
     case IrOpcode::kSelect:
@@ -454,6 +456,30 @@ Reduction TypedOptimization::ReduceStringComparison(Node* node) {
   return NoChange();
 }
 
+Reduction TypedOptimization::ReduceStringLength(Node* node) {
+  DCHECK_EQ(IrOpcode::kStringLength, node->opcode());
+  Node* const input = NodeProperties::GetValueInput(node, 0);
+  switch (input->opcode()) {
+    case IrOpcode::kHeapConstant: {
+      // Constant-fold the String::length of the {input}.
+      HeapObjectMatcher m(input);
+      if (m.Ref(js_heap_broker()).IsString()) {
+        uint32_t const length = m.Ref(js_heap_broker()).AsString().length();
+        Node* value = jsgraph()->Constant(length);
+        return Replace(value);
+      }
+      break;
+    }
+    case IrOpcode::kStringConcat: {
+      // The first value input to the {input} is the resulting length.
+      return Replace(input->InputAt(0));
+    }
+    default:
+      break;
+  }
+  return NoChange();
+}
+
 Reduction TypedOptimization::ReduceSameValue(Node* node) {
   DCHECK_EQ(IrOpcode::kSameValue, node->opcode());
   Node* const lhs = NodeProperties::GetValueInput(node, 0);

src/compiler/typed-optimization.h

@@ -50,6 +50,7 @@ class V8_EXPORT_PRIVATE TypedOptimization final
   Reduction ReducePhi(Node* node);
   Reduction ReduceReferenceEqual(Node* node);
   Reduction ReduceStringComparison(Node* node);
+  Reduction ReduceStringLength(Node* node);
   Reduction ReduceSameValue(Node* node);
   Reduction ReduceSelect(Node* node);
   Reduction ReduceSpeculativeToNumber(Node* node);

src/compiler/typer.cc

@@ -1864,6 +1864,8 @@ Type Typer::Visitor::TypeSpeculativeNumberLessThanOrEqual(Node* node) {
   return TypeBinaryOp(node, NumberLessThanOrEqualTyper);
 }
 
+Type Typer::Visitor::TypeStringConcat(Node* node) { return Type::String(); }
+
 Type Typer::Visitor::TypeStringToNumber(Node* node) {
   return TypeUnaryOp(node, ToNumber);
 }
@@ -2000,8 +2002,6 @@ Type Typer::Visitor::TypeCheckSymbol(Node* node) {
   return Type::Intersect(arg, Type::Symbol(), zone());
 }
 
-Type Typer::Visitor::TypeCheckStringAdd(Node* node) { return Type::String(); }
-
 Type Typer::Visitor::TypeCheckFloat64Hole(Node* node) {
   return typer_->operation_typer_.CheckFloat64Hole(Operand(node, 0));
 }

src/compiler/verifier.cc

@@ -1094,6 +1094,12 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) {
       CheckValueInputIs(node, 0, Type::PlainPrimitive());
       CheckTypeIs(node, Type::Number());
       break;
+    case IrOpcode::kStringConcat:
+      CheckValueInputIs(node, 0, TypeCache::Get().kStringLengthType);
+      CheckValueInputIs(node, 1, Type::String());
+      CheckValueInputIs(node, 2, Type::String());
+      CheckTypeIs(node, Type::String());
+      break;
     case IrOpcode::kStringEqual:
     case IrOpcode::kStringLessThan:
     case IrOpcode::kStringLessThanOrEqual:
@@ -1434,11 +1440,6 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) {
       CheckValueInputIs(node, 0, Type::Any());
       CheckTypeIs(node, Type::Symbol());
       break;
-    case IrOpcode::kCheckStringAdd:
-      CheckValueInputIs(node, 0, Type::String());
-      CheckValueInputIs(node, 1, Type::String());
-      CheckTypeIs(node, Type::String());
-      break;
     case IrOpcode::kConvertReceiver:
       // (Any, Any) -> Receiver
       CheckValueInputIs(node, 0, Type::Any());

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

@@ -401,7 +401,7 @@ TEST_F(JSTypedLoweringTest, JSAddWithString) {
   Reduction r = Reduce(graph()->NewNode(javascript()->Add(hint), lhs, rhs,
                                         context, frame_state, effect, control));
   ASSERT_TRUE(r.Changed());
-  EXPECT_THAT(r.replacement(), IsCheckStringAdd(lhs, rhs));
+  EXPECT_THAT(r.replacement(), IsStringConcat(_, lhs, rhs));
 }
 
 }  // namespace compiler

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

@@ -1401,6 +1401,43 @@ class IsBinopMatcher final : public TestNodeMatcher {
   const Matcher<Node*> rhs_matcher_;
 };
 
+class IsStringConcatMatcher final : public TestNodeMatcher {
+ public:
+  IsStringConcatMatcher(const Matcher<Node*>& length_matcher,
+                        const Matcher<Node*>& lhs_matcher,
+                        const Matcher<Node*>& rhs_matcher)
+      : TestNodeMatcher(IrOpcode::kStringConcat),
+        length_matcher_(length_matcher),
+        lhs_matcher_(lhs_matcher),
+        rhs_matcher_(rhs_matcher) {}
+
+  void DescribeTo(std::ostream* os) const final {
+    TestNodeMatcher::DescribeTo(os);
+    *os << " whose length (";
+    length_matcher_.DescribeTo(os);
+    *os << ") and lhs (";
+    lhs_matcher_.DescribeTo(os);
+    *os << ") and rhs (";
+    rhs_matcher_.DescribeTo(os);
+    *os << ")";
+  }
+
+  bool MatchAndExplain(Node* node, MatchResultListener* listener) const final {
+    return (TestNodeMatcher::MatchAndExplain(node, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetValueInput(node, 0),
+                                 "length", length_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetValueInput(node, 1), "lhs",
+                                 lhs_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetValueInput(node, 2), "rhs",
+                                 rhs_matcher_, listener));
+  }
+
+ private:
+  const Matcher<Node*> length_matcher_;
+  const Matcher<Node*> lhs_matcher_;
+  const Matcher<Node*> rhs_matcher_;
+};
+
 class IsUnopMatcher final : public TestNodeMatcher {
  public:
   IsUnopMatcher(IrOpcode::Value opcode, const Matcher<Node*>& input_matcher)
@@ -1913,6 +1950,13 @@ Matcher<Node*> IsTailCall(
 SPECULATIVE_BINOPS(DEFINE_SPECULATIVE_BINOP_MATCHER);
 #undef DEFINE_SPECULATIVE_BINOP_MATCHER
 
+Matcher<Node*> IsStringConcat(const Matcher<Node*>& length_matcher,
+                              const Matcher<Node*>& lhs_matcher,
+                              const Matcher<Node*>& rhs_matcher) {
+  return MakeMatcher(
+      new IsStringConcatMatcher(length_matcher, lhs_matcher, rhs_matcher));
+}
+
 Matcher<Node*> IsAllocate(const Matcher<Node*>& size_matcher,
                           const Matcher<Node*>& effect_matcher,
                           const Matcher<Node*>& control_matcher) {
@@ -2126,7 +2170,6 @@ IS_BINOP_MATCHER(Float64Sub)
 IS_BINOP_MATCHER(Float64Mul)
 IS_BINOP_MATCHER(Float64InsertLowWord32)
 IS_BINOP_MATCHER(Float64InsertHighWord32)
-IS_BINOP_MATCHER(CheckStringAdd)
 #undef IS_BINOP_MATCHER
 
 

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

@@ -272,6 +272,9 @@ Matcher<Node*> IsNumberSqrt(const Matcher<Node*>& value_matcher);
 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*> IsStringConcat(const Matcher<Node*>& length_matcher,
+                              const Matcher<Node*>& lhs_matcher,
+                              const Matcher<Node*>& rhs_matcher);
 Matcher<Node*> IsStringFromSingleCharCode(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsStringLength(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsAllocate(const Matcher<Node*>& size_matcher,
@@ -496,9 +499,6 @@ Matcher<Node*> IsStackSlot();
 
 Matcher<Node*> IsSpeculativeToNumber(const Matcher<Node*>& value_matcher);
 
-Matcher<Node*> IsCheckStringAdd(const Matcher<Node*>& lhs_matcher,
-                                const Matcher<Node*>& rhs_matcher);
-
 // Helpers
 static inline Matcher<Node*> IsIntPtrConstant(const intptr_t value) {
   return kPointerSize == 8 ? IsInt64Constant(static_cast<int64_t>(value))