- Revision
- 230964
- Author
- fpi...@apple.com
- Date
- 2018-04-24 11:54:47 -0700 (Tue, 24 Apr 2018)
Log Message
DFG CSE should know how to decay a MultiGetByOffset
https://bugs.webkit.org/show_bug.cgi?id=159859
Reviewed by Keith Miller.
This teaches Node::remove() how to decay a MultiGetByOffset to a CheckStructure, so that
clobberize() can report a def() for MultiGetByOffset.
This is a slight improvement to codegen in splay because splay is a heavy user of
MultiGetByOffset. It uses it redundantly in one of its hot functions (the function called
"splay_"). I don't see a net speed-up in the benchmark. However, this is just a first step to
removing MultiXByOffset-related redundancies, which by my estimates account for 16% of
splay's time.
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGNode.cpp:
(JSC::DFG::Node::remove):
(JSC::DFG::Node::removeWithoutChecks):
(JSC::DFG::Node::replaceWith):
(JSC::DFG::Node::replaceWithWithoutChecks):
* dfg/DFGNode.h:
(JSC::DFG::Node::convertToMultiGetByOffset):
(JSC::DFG::Node::replaceWith): Deleted.
* dfg/DFGNodeType.h:
* dfg/DFGObjectAllocationSinkingPhase.cpp:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (230963 => 230964)
--- trunk/Source/_javascript_Core/ChangeLog 2018-04-24 18:15:48 UTC (rev 230963)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-04-24 18:54:47 UTC (rev 230964)
@@ -1,3 +1,32 @@
+2018-04-24 Filip Pizlo <fpi...@apple.com>
+
+ DFG CSE should know how to decay a MultiGetByOffset
+ https://bugs.webkit.org/show_bug.cgi?id=159859
+
+ Reviewed by Keith Miller.
+
+ This teaches Node::remove() how to decay a MultiGetByOffset to a CheckStructure, so that
+ clobberize() can report a def() for MultiGetByOffset.
+
+ This is a slight improvement to codegen in splay because splay is a heavy user of
+ MultiGetByOffset. It uses it redundantly in one of its hot functions (the function called
+ "splay_"). I don't see a net speed-up in the benchmark. However, this is just a first step to
+ removing MultiXByOffset-related redundancies, which by my estimates account for 16% of
+ splay's time.
+
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGNode.cpp:
+ (JSC::DFG::Node::remove):
+ (JSC::DFG::Node::removeWithoutChecks):
+ (JSC::DFG::Node::replaceWith):
+ (JSC::DFG::Node::replaceWithWithoutChecks):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::convertToMultiGetByOffset):
+ (JSC::DFG::Node::replaceWith): Deleted.
+ * dfg/DFGNodeType.h:
+ * dfg/DFGObjectAllocationSinkingPhase.cpp:
+
2018-04-24 Keith Miller <keith_mil...@apple.com>
Update API docs with information on which run loop the VM will use
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (230963 => 230964)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2018-04-24 18:15:48 UTC (rev 230963)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2018-04-24 18:54:47 UTC (rev 230964)
@@ -1188,9 +1188,7 @@
read(JSObject_butterflyMask);
AbstractHeap heap(NamedProperties, node->multiGetByOffsetData().identifierNumber);
read(heap);
- // FIXME: We cannot def() for MultiGetByOffset because CSE is not smart enough to decay it
- // to a CheckStructure.
- // https://bugs.webkit.org/show_bug.cgi?id=159859
+ def(HeapLocation(NamedPropertyLoc, heap, node->child1()), LazyNode(node));
return;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.cpp (230963 => 230964)
--- trunk/Source/_javascript_Core/dfg/DFGNode.cpp 2018-04-24 18:15:48 UTC (rev 230963)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.cpp 2018-04-24 18:54:47 UTC (rev 230964)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013, 2014, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -83,27 +83,62 @@
void Node::remove(Graph& graph)
{
- if (flags() & NodeHasVarArgs) {
- unsigned targetIndex = 0;
- for (unsigned i = 0; i < numChildren(); ++i) {
- Edge& edge = graph.varArgChild(this, i);
- if (!edge)
- continue;
- if (edge.willHaveCheck()) {
- Edge& dst = graph.varArgChild(this, targetIndex++);
- std::swap(dst, edge);
- continue;
+ switch (op()) {
+ case MultiGetByOffset: {
+ MultiGetByOffsetData& data = ""
+ StructureSet set;
+ for (MultiGetByOffsetCase& getCase : data.cases) {
+ getCase.set().forEach(
+ [&] (RegisteredStructure structure) {
+ set.add(structure.get());
+ });
+ }
+ convertToCheckStructure(graph.addStructureSet(set));
+ return;
+ }
+
+ default:
+ if (flags() & NodeHasVarArgs) {
+ unsigned targetIndex = 0;
+ for (unsigned i = 0; i < numChildren(); ++i) {
+ Edge& edge = graph.varArgChild(this, i);
+ if (!edge)
+ continue;
+ if (edge.willHaveCheck()) {
+ Edge& dst = graph.varArgChild(this, targetIndex++);
+ std::swap(dst, edge);
+ continue;
+ }
+ edge = Edge();
}
- edge = Edge();
+ setOpAndDefaultFlags(CheckVarargs);
+ children.setNumChildren(targetIndex);
+ } else {
+ children = children.justChecks();
+ setOpAndDefaultFlags(Check);
}
- setOpAndDefaultFlags(CheckVarargs);
- children.setNumChildren(targetIndex);
- } else {
- children = children.justChecks();
- setOpAndDefaultFlags(Check);
+ return;
}
}
+void Node::removeWithoutChecks()
+{
+ children = AdjacencyList();
+ setOpAndDefaultFlags(Check);
+}
+
+void Node::replaceWith(Graph& graph, Node* other)
+{
+ remove(graph);
+ setReplacement(other);
+}
+
+void Node::replaceWithWithoutChecks(Node* other)
+{
+ removeWithoutChecks();
+ setReplacement(other);
+}
+
void Node::convertToIdentity()
{
RELEASE_ASSERT(child1());
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (230963 => 230964)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2018-04-24 18:15:48 UTC (rev 230963)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2018-04-24 18:54:47 UTC (rev 230964)
@@ -431,6 +431,7 @@
}
void remove(Graph&);
+ void removeWithoutChecks();
void convertToCheckStructure(RegisteredStructureSet* set)
{
@@ -451,11 +452,8 @@
children.setChild1(Edge(structure, CellUse));
}
- void replaceWith(Graph& graph, Node* other)
- {
- remove(graph);
- setReplacement(other);
- }
+ void replaceWith(Graph&, Node* other);
+ void replaceWithWithoutChecks(Node* other);
void convertToIdentity();
void convertToIdentityOn(Node*);
@@ -562,11 +560,11 @@
void convertToMultiGetByOffset(MultiGetByOffsetData* data)
{
- ASSERT(m_op == GetById || m_op == GetByIdFlush || m_op == GetByIdDirect || m_op == GetByIdDirectFlush);
+ RELEASE_ASSERT(m_op == GetById || m_op == GetByIdFlush || m_op == GetByIdDirect || m_op == GetByIdDirectFlush);
m_opInfo = data;
child1().setUseKind(CellUse);
m_op = MultiGetByOffset;
- ASSERT(m_flags & NodeMustGenerate);
+ RELEASE_ASSERT(m_flags & NodeMustGenerate);
}
void convertToPutByOffset(StorageAccessData& data, Edge storage, Edge base)
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (230963 => 230964)
--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2018-04-24 18:15:48 UTC (rev 230963)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2018-04-24 18:54:47 UTC (rev 230964)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Modified: trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp (230963 => 230964)
--- trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2018-04-24 18:15:48 UTC (rev 230963)
+++ trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2018-04-24 18:54:47 UTC (rev 230964)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -960,7 +960,7 @@
PromotedHeapLocation location(NamedPropertyPLoc, allocation->identifier(), identifierNumber);
if (Node* value = heapResolve(location)) {
if (allocation->structures().isSubsetOf(validStructures))
- node->replaceWith(m_graph, value);
+ node->replaceWithWithoutChecks(value);
else {
Node* structure = heapResolve(PromotedHeapLocation(allocation->identifier(), StructurePLoc));
ASSERT(structure);
