[turbofan] Enable typed lowering of string addition.

Unfortunately StringAdd is not pure in V8 because we might throw an
exception if the resulting string length is outside the valid bounds, so
there's no point in having a simplified StringAdd operator.

R=jarin@chromium.org

Review URL: https://codereview.chromium.org/1164743002

Cr-Commit-Position: refs/heads/master@{#28747}

diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc
index 61f23ca..4a4bcbf 100644 (file)
--- a/src/compiler/js-typed-lowering.cc
+++ b/src/compiler/js-typed-lowering.cc
@@ -370,20 +370,20 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
     r.ConvertInputsToNumber(frame_state);
     return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number());
   }
-#if 0
-  // TODO(turbofan): Lowering of StringAdd is disabled for now because:
-  //   a) The inserted ToString operation screws up valueOf vs. toString order.
-  //   b) Deoptimization at ToString doesn't have corresponding bailout id.
-  //   c) Our current StringAddStub is actually non-pure and requires context.
-  if ((r.OneInputIs(Type::String()) && !r.IsStrong()) ||
-      r.BothInputsAre(Type::String())) {
-    // JSAdd(x:string, y:string) => StringAdd(x, y)
-    // JSAdd(x:string, y) => StringAdd(x, ToString(y))
-    // JSAdd(x, y:string) => StringAdd(ToString(x), y)
-    r.ConvertInputsToString();
-    return r.ChangeToPureOperator(simplified()->StringAdd());
-  }
-#endif
+  if (r.BothInputsAre(Type::String())) {
+    // JSAdd(x:string, y:string) => CallStub[StringAdd](x, y)
+    Callable const callable =
+        CodeFactory::StringAdd(isolate(), STRING_ADD_CHECK_NONE, NOT_TENURED);
+    CallDescriptor const* const desc = Linkage::GetStubCallDescriptor(
+        isolate(), graph()->zone(), callable.descriptor(), 0,
+        CallDescriptor::kNeedsFrameState, node->op()->properties());
+    DCHECK_EQ(2, OperatorProperties::GetFrameStateInputCount(node->op()));
+    node->RemoveInput(NodeProperties::FirstFrameStateIndex(node) + 1);
+    node->InsertInput(graph()->zone(), 0,
+                      jsgraph()->HeapConstant(callable.code()));
+    node->set_op(common()->Call(desc));
+    return Changed(node);
+  }
   return NoChange();
 }
 
@@ -1467,6 +1467,9 @@ Factory* JSTypedLowering::factory() const { return jsgraph()->factory(); }
 Graph* JSTypedLowering::graph() const { return jsgraph()->graph(); }
 
 
+Isolate* JSTypedLowering::isolate() const { return jsgraph()->isolate(); }
+
+
 JSOperatorBuilder* JSTypedLowering::javascript() const {
   return jsgraph()->javascript();
 }

diff --git a/src/compiler/js-typed-lowering.h b/src/compiler/js-typed-lowering.h
index 64931e1..add45ce 100644 (file)
--- a/src/compiler/js-typed-lowering.h
+++ b/src/compiler/js-typed-lowering.h
@@ -72,6 +72,7 @@ class JSTypedLowering final : public AdvancedReducer {
   Factory* factory() const;
   Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }
+  Isolate* isolate() const;
   JSOperatorBuilder* javascript() const;
   CommonOperatorBuilder* common() const;
   SimplifiedOperatorBuilder* simplified() { return &simplified_; }

diff --git a/src/compiler/opcodes.h b/src/compiler/opcodes.h
index 4a1e85f..d38f24d 100644 (file)
--- a/src/compiler/opcodes.h
+++ b/src/compiler/opcodes.h
@@ -172,7 +172,6 @@
   V(NumberToInt32)                 \
   V(NumberToUint32)                \
   V(PlainPrimitiveToNumber)        \
-  V(StringAdd)                     \
   V(ChangeTaggedToInt32)           \
   V(ChangeTaggedToUint32)          \
   V(ChangeTaggedToFloat64)         \

diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc
index 929f8fd..64f7e22 100644 (file)
--- a/src/compiler/simplified-lowering.cc
+++ b/src/compiler/simplified-lowering.cc
@@ -782,11 +782,6 @@ class RepresentationSelector {
         if (lower()) lowering->DoStringLessThanOrEqual(node);
         break;
       }
-      case IrOpcode::kStringAdd: {
-        VisitBinop(node, kMachAnyTagged, kMachAnyTagged);
-        if (lower()) lowering->DoStringAdd(node);
-        break;
-      }
       case IrOpcode::kAllocate: {
         ProcessInput(node, 0, kMachAnyTagged);
         ProcessRemainingInputs(node, 1);
@@ -1313,23 +1308,6 @@ void SimplifiedLowering::DoStoreElement(Node* node) {
 }
 
 
-void SimplifiedLowering::DoStringAdd(Node* node) {
-  Operator::Properties properties = node->op()->properties();
-  Callable callable = CodeFactory::StringAdd(
-      jsgraph()->isolate(), STRING_ADD_CHECK_NONE, NOT_TENURED);
-  CallDescriptor::Flags flags = CallDescriptor::kNoFlags;
-  CallDescriptor* desc = Linkage::GetStubCallDescriptor(
-      jsgraph()->isolate(), zone(), callable.descriptor(), 0, flags,
-      properties);
-  node->set_op(common()->Call(desc));
-  node->InsertInput(graph()->zone(), 0,
-                    jsgraph()->HeapConstant(callable.code()));
-  node->AppendInput(graph()->zone(), jsgraph()->UndefinedConstant());
-  node->AppendInput(graph()->zone(), graph()->start());
-  node->AppendInput(graph()->zone(), graph()->start());
-}
-
-
 Node* SimplifiedLowering::StringComparison(Node* node, bool requires_ordering) {
   Runtime::FunctionId f =
       requires_ordering ? Runtime::kStringCompareRT : Runtime::kStringEquals;

diff --git a/src/compiler/simplified-lowering.h b/src/compiler/simplified-lowering.h
index 124090e..d6e17e7 100644 (file)
--- a/src/compiler/simplified-lowering.h
+++ b/src/compiler/simplified-lowering.h
@@ -38,7 +38,6 @@ class SimplifiedLowering final {
   void DoStoreBuffer(Node* node);
   void DoLoadElement(Node* node);
   void DoStoreElement(Node* node);
-  void DoStringAdd(Node* node);
   void DoStringEqual(Node* node);
   void DoStringLessThan(Node* node);
   void DoStringLessThanOrEqual(Node* node);

diff --git a/src/compiler/simplified-operator.cc b/src/compiler/simplified-operator.cc
index 9b34668..33cedd0 100644 (file)
--- a/src/compiler/simplified-operator.cc
+++ b/src/compiler/simplified-operator.cc
@@ -174,7 +174,6 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
   V(StringEqual, Operator::kCommutative, 2)             \
   V(StringLessThan, Operator::kNoProperties, 2)         \
   V(StringLessThanOrEqual, Operator::kNoProperties, 2)  \
-  V(StringAdd, Operator::kNoProperties, 2)              \
   V(ChangeTaggedToInt32, Operator::kNoProperties, 1)    \
   V(ChangeTaggedToUint32, Operator::kNoProperties, 1)   \
   V(ChangeTaggedToFloat64, Operator::kNoProperties, 1)  \

diff --git a/src/compiler/simplified-operator.h b/src/compiler/simplified-operator.h
index 484b39b..8f271f6 100644 (file)
--- a/src/compiler/simplified-operator.h
+++ b/src/compiler/simplified-operator.h
@@ -149,7 +149,6 @@ class SimplifiedOperatorBuilder final {
   const Operator* StringEqual();
   const Operator* StringLessThan();
   const Operator* StringLessThanOrEqual();
-  const Operator* StringAdd();
 
   const Operator* ChangeTaggedToInt32();
   const Operator* ChangeTaggedToUint32();

diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc
index 83d444c..f0835e7 100644 (file)
--- a/src/compiler/typer.cc
+++ b/src/compiler/typer.cc
@@ -1729,11 +1729,6 @@ Bounds Typer::Visitor::TypeStringLessThanOrEqual(Node* node) {
 }
 
 
-Bounds Typer::Visitor::TypeStringAdd(Node* node) {
-  return Bounds(Type::None(zone()), Type::String(zone()));
-}
-
-
 namespace {
 
 Type* ChangeRepresentation(Type* type, Type* rep, Zone* zone) {

diff --git a/src/compiler/verifier.cc b/src/compiler/verifier.cc
index 4e8c23a..6424fa6 100644 (file)
--- a/src/compiler/verifier.cc
+++ b/src/compiler/verifier.cc
@@ -644,12 +644,6 @@ void Verifier::Visitor::Check(Node* node) {
       CheckValueInputIs(node, 1, Type::String());
       CheckUpperIs(node, Type::Boolean());
       break;
-    case IrOpcode::kStringAdd:
-      // (String, String) -> String
-      CheckValueInputIs(node, 0, Type::String());
-      CheckValueInputIs(node, 1, Type::String());
-      CheckUpperIs(node, Type::String());
-      break;
     case IrOpcode::kReferenceEqual: {
       // (Unique, Any) -> Boolean  and
       // (Any, Unique) -> Boolean

diff --git a/test/cctest/compiler/simplified-graph-builder.h b/test/cctest/compiler/simplified-graph-builder.h
index c9ba002..50c51d5 100644 (file)
--- a/test/cctest/compiler/simplified-graph-builder.h
+++ b/test/cctest/compiler/simplified-graph-builder.h
@@ -92,9 +92,6 @@ class SimplifiedGraphBuilder : public GraphBuilder {
   Node* StringLessThanOrEqual(Node* a, Node* b) {
     return NewNode(simplified()->StringLessThanOrEqual(), a, b);
   }
-  Node* StringAdd(Node* a, Node* b) {
-    return NewNode(simplified()->StringAdd(), a, b);
-  }
 
   Node* ChangeTaggedToInt32(Node* a) {
     return NewNode(simplified()->ChangeTaggedToInt32(), a);

diff --git a/test/cctest/compiler/test-simplified-lowering.cc b/test/cctest/compiler/test-simplified-lowering.cc
index 9b23469..bc2634e 100644 (file)
--- a/test/cctest/compiler/test-simplified-lowering.cc
+++ b/test/cctest/compiler/test-simplified-lowering.cc
@@ -1266,7 +1266,6 @@ TEST(LowerStringOps_to_call_and_compare) {
     t.CheckLoweringBinop(compare_eq, t.simplified()->StringEqual());
     t.CheckLoweringBinop(compare_lt, t.simplified()->StringLessThan());
     t.CheckLoweringBinop(compare_le, t.simplified()->StringLessThanOrEqual());
-    t.CheckLoweringBinop(IrOpcode::kCall, t.simplified()->StringAdd());
   }
 }
 

diff --git a/test/unittests/compiler/js-typed-lowering-unittest.cc b/test/unittests/compiler/js-typed-lowering-unittest.cc
index ddff925..8a0cb17 100644 (file)
--- a/test/unittests/compiler/js-typed-lowering-unittest.cc
+++ b/test/unittests/compiler/js-typed-lowering-unittest.cc
@@ -86,12 +86,6 @@ class JSTypedLoweringTest : public TypedGraphTest {
     return reducer.Reduce(node);
   }
 
-  Node* EmptyFrameState() {
-    MachineOperatorBuilder machine(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
-    return jsgraph.EmptyFrameState();
-  }
-
   Handle<JSArrayBuffer> NewArrayBuffer(void* bytes, size_t byte_length) {
     Handle<JSArrayBuffer> buffer = factory()->NewJSArrayBuffer();
     Runtime::SetupArrayBuffer(isolate(), buffer, true, bytes, byte_length);
@@ -902,11 +896,39 @@ TEST_F(JSTypedLoweringTest, JSLoadNamedGlobalConstants) {
   }
 }
 
+#if V8_TURBOFAN_TARGET
+
+// -----------------------------------------------------------------------------
+// JSAdd
+
+
+TEST_F(JSTypedLoweringTest, JSAddWithString) {
+  TRACED_FOREACH(LanguageMode, language_mode, kLanguageModes) {
+    Node* lhs = Parameter(Type::String(), 0);
+    Node* rhs = Parameter(Type::String(), 1);
+    Node* context = Parameter(Type::Any(), 2);
+    Node* frame_state0 = EmptyFrameState();
+    Node* frame_state1 = EmptyFrameState();
+    Node* effect = graph()->start();
+    Node* control = graph()->start();
+    Reduction r = Reduce(graph()->NewNode(javascript()->Add(language_mode), lhs,
+                                          rhs, context, frame_state0,
+                                          frame_state1, effect, control));
+    ASSERT_TRUE(r.Changed());
+    EXPECT_THAT(
+        r.replacement(),
+        IsCall(_, IsHeapConstant(Unique<HeapObject>::CreateImmovable(
+                      CodeFactory::StringAdd(isolate(), STRING_ADD_CHECK_NONE,
+                                             NOT_TENURED).code())),
+               lhs, rhs, context, frame_state0, effect, control));
+  }
+}
+
 
 // -----------------------------------------------------------------------------
 // JSCreateClosure
 
-#if V8_TURBOFAN_TARGET
+
 TEST_F(JSTypedLoweringTest, JSCreateClosure) {
   Node* const context = UndefinedConstant();
   Node* const effect = graph()->start();
@@ -973,7 +995,8 @@ TEST_F(JSTypedLoweringTest, JSCreateLiteralObject) {
                     CodeFactory::FastCloneShallowObject(isolate(), 6).code())),
              input0, input1, input2, _, context, frame_state, effect, control));
 }
-#endif
+
+#endif  // V8_TURBOFAN_TARGET
 
 
 // -----------------------------------------------------------------------------

diff --git a/test/unittests/compiler/simplified-operator-unittest.cc b/test/unittests/compiler/simplified-operator-unittest.cc
index a5dad5a..0772891 100644 (file)
--- a/test/unittests/compiler/simplified-operator-unittest.cc
+++ b/test/unittests/compiler/simplified-operator-unittest.cc
@@ -54,7 +54,6 @@ const PureOperator kPureOperators[] = {
     PURE(StringEqual, Operator::kCommutative, 2),
     PURE(StringLessThan, Operator::kNoProperties, 2),
     PURE(StringLessThanOrEqual, Operator::kNoProperties, 2),
-    PURE(StringAdd, Operator::kNoProperties, 2),
     PURE(ChangeTaggedToInt32, Operator::kNoProperties, 1),
     PURE(ChangeTaggedToUint32, Operator::kNoProperties, 1),
     PURE(ChangeTaggedToFloat64, Operator::kNoProperties, 1),