DFG implementation of op_strcat should inline rope allocations
https://bugs.webkit.org/show_bug.cgi?id=112780
Reviewed by Oliver Hunt.
This gets rid of the StrCat node and adds a MakeRope node. The MakeRope node can
take either two or three operands, and allocates a rope string with either two or
three fibers. (The magic choice of three children for non-VarArg nodes happens to
match exactly with the magic choice of three fibers for rope strings.)
ValueAdd on KnownString is replaced with MakeRope with two children.
StrCat gets replaced by an appropriate sequence of MakeRope's.
MakeRope does not do the dynamic check to see if its children are empty strings.
This is replaced by a static check, instead. The downside is that we may use more
memory if the strings passed to MakeRope turn out to dynamically be empty. The
upside is that we do fewer checks in the cases where either the strings are not
empty, or where the strings are statically known to be empty. I suspect both of
those cases are more common, than the case where the string is dynamically empty.
This also results in some badness for X86. MakeRope needs six registers if it is
allocating a three-rope. We don't have six registers to spare on X86. Currently,
the code side-steps this problem by just never usign three-ropes in optimized
code on X86. All other architectures, including X86_64, don't have this problem.
This is a shocking speed-up. 9% progressions on both V8/splay and
SunSpider/date-format-xparb. 1% progression on V8v7 overall, and ~0.5% progression
on SunSpider. 2x speed-up on microbenchmarks that test op_strcat.
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::executeEffects):
* dfg/DFGAdjacencyList.h:
(AdjacencyList):
(JSC::DFG::AdjacencyList::removeEdge):
* dfg/DFGArgumentsSimplificationPhase.cpp:
(JSC::DFG::ArgumentsSimplificationPhase::removeArgumentsReferencingPhantomChild):
* dfg/DFGBackwardsPropagationPhase.cpp:
(JSC::DFG::BackwardsPropagationPhase::propagate):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCSEPhase.cpp:
(JSC::DFG::CSEPhase::putStructureStoreElimination):
(JSC::DFG::CSEPhase::eliminateIrrelevantPhantomChildren):
(JSC::DFG::CSEPhase::performNodeCSE):
* dfg/DFGDCEPhase.cpp:
(JSC::DFG::DCEPhase::eliminateIrrelevantPhantomChildren):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::createToString):
(JSC::DFG::FixupPhase::attemptToForceStringArrayModeByToStringConversion):
(JSC::DFG::FixupPhase::convertStringAddUse):
(FixupPhase):
(JSC::DFG::FixupPhase::convertToMakeRope):
(JSC::DFG::FixupPhase::fixupMakeRope):
(JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
* dfg/DFGNodeType.h:
(DFG):
* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileAdd):
(JSC::DFG::SpeculativeJIT::compileMakeRope):
(DFG):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::callOperation):
(SpeculativeJIT):
(JSC::DFG::SpeculateCellOperand::SpeculateCellOperand):
(JSC::DFG::SpeculateCellOperand::~SpeculateCellOperand):
(JSC::DFG::SpeculateCellOperand::gpr):
(JSC::DFG::SpeculateCellOperand::use):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* runtime/JSString.h:
(JSRopeString):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@146382 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index 39861fa..30318f3 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -175,6 +175,11 @@
break;
}
+ case MakeRope: {
+ fixupMakeRope(node);
+ break;
+ }
+
case ArithAdd:
case ArithSub: {
if (attemptToMakeIntegerAdd(node))
@@ -862,7 +867,6 @@
case IsString:
case IsObject:
case IsFunction:
- case StrCat:
case CreateActivation:
case TearOffActivation:
case CreateArguments:
@@ -898,11 +902,11 @@
}
template<UseKind useKind>
- Node* createToString(Node* node, Edge edge)
+ void createToString(Node* node, Edge& edge)
{
- return m_insertionSet.insertNode(
+ edge.setNode(m_insertionSet.insertNode(
m_indexInBlock, SpecString, ToString, node->codeOrigin,
- Edge(edge.node(), useKind));
+ Edge(edge.node(), useKind)));
}
template<UseKind useKind>
@@ -913,7 +917,7 @@
if (!canOptimizeStringObjectAccess(node->codeOrigin))
return;
- node->child1().setNode(createToString<useKind>(node, node->child1()));
+ createToString<useKind>(node, node->child1());
arrayMode = ArrayMode(Array::String);
}
@@ -947,7 +951,34 @@
// FIXME: We ought to be able to have a ToPrimitiveToString node.
observeUseKindOnNode<useKind>(edge.node());
- edge = Edge(createToString<useKind>(node, edge), KnownStringUse);
+ createToString<useKind>(node, edge);
+ }
+
+ void convertToMakeRope(Node* node)
+ {
+ node->setOpAndDefaultFlags(MakeRope);
+ fixupMakeRope(node);
+ }
+
+ void fixupMakeRope(Node* node)
+ {
+ for (unsigned i = 0; i < AdjacencyList::Size; ++i) {
+ Edge& edge = node->children.child(i);
+ if (!edge)
+ break;
+ edge.setUseKind(KnownStringUse);
+ if (!m_graph.isConstant(edge.node()))
+ continue;
+ JSString* string = jsCast<JSString*>(m_graph.valueOfJSConstant(edge.node()).asCell());
+ if (string->length())
+ continue;
+ node->children.removeEdge(i--);
+ }
+
+ if (!node->child2()) {
+ ASSERT(!node->child3());
+ node->convertToIdentity();
+ }
}
template<UseKind leftUseKind>
@@ -961,6 +992,7 @@
if (right->shouldSpeculateString()) {
convertStringAddUse<leftUseKind>(node, left);
convertStringAddUse<StringUse>(node, right);
+ convertToMakeRope(node);
return true;
}
@@ -968,6 +1000,7 @@
&& canOptimizeStringObjectAccess(node->codeOrigin)) {
convertStringAddUse<leftUseKind>(node, left);
convertStringAddUse<StringObjectUse>(node, right);
+ convertToMakeRope(node);
return true;
}
@@ -975,6 +1008,7 @@
&& canOptimizeStringObjectAccess(node->codeOrigin)) {
convertStringAddUse<leftUseKind>(node, left);
convertStringAddUse<StringOrStringObjectUse>(node, right);
+ convertToMakeRope(node);
return true;
}
