- Revision
- 140504
- Author
- [email protected]
- Date
- 2013-01-22 21:35:23 -0800 (Tue, 22 Jan 2013)
Log Message
Convert CSE phase to not rely too much on NodeIndex
https://bugs.webkit.org/show_bug.cgi?id=107616
Reviewed by Geoffrey Garen.
- Instead of looping over the graph (which assumes that you can simply loop over all
nodes without considering blocks first) to reset node.replacement, do that in the
loop that sets up relevantToOSR, just before running CSE on the block.
- Instead of having a relevantToOSR bitvector indexed by NodeIndex, made
NodeRelevantToOSR be a NodeFlag. We had exactly one bit left in NodeFlags, so I did
some reshuffling to fit it in.
* dfg/DFGCSEPhase.cpp:
(JSC::DFG::CSEPhase::CSEPhase):
(JSC::DFG::CSEPhase::eliminateIrrelevantPhantomChildren):
(JSC::DFG::CSEPhase::performNodeCSE):
(JSC::DFG::CSEPhase::performBlockCSE):
(CSEPhase):
* dfg/DFGNodeFlags.h:
(DFG):
* dfg/DFGNodeType.h:
(DFG):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (140503 => 140504)
--- trunk/Source/_javascript_Core/ChangeLog 2013-01-23 05:14:42 UTC (rev 140503)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-01-23 05:35:23 UTC (rev 140504)
@@ -1,3 +1,29 @@
+2013-01-22 Filip Pizlo <[email protected]>
+
+ Convert CSE phase to not rely too much on NodeIndex
+ https://bugs.webkit.org/show_bug.cgi?id=107616
+
+ Reviewed by Geoffrey Garen.
+
+ - Instead of looping over the graph (which assumes that you can simply loop over all
+ nodes without considering blocks first) to reset node.replacement, do that in the
+ loop that sets up relevantToOSR, just before running CSE on the block.
+
+ - Instead of having a relevantToOSR bitvector indexed by NodeIndex, made
+ NodeRelevantToOSR be a NodeFlag. We had exactly one bit left in NodeFlags, so I did
+ some reshuffling to fit it in.
+
+ * dfg/DFGCSEPhase.cpp:
+ (JSC::DFG::CSEPhase::CSEPhase):
+ (JSC::DFG::CSEPhase::eliminateIrrelevantPhantomChildren):
+ (JSC::DFG::CSEPhase::performNodeCSE):
+ (JSC::DFG::CSEPhase::performBlockCSE):
+ (CSEPhase):
+ * dfg/DFGNodeFlags.h:
+ (DFG):
+ * dfg/DFGNodeType.h:
+ (DFG):
+
2013-01-21 Kentaro Hara <[email protected]>
Implement UIEvent constructor
Modified: trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp (140503 => 140504)
--- trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2013-01-23 05:14:42 UTC (rev 140503)
+++ trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2013-01-23 05:35:23 UTC (rev 140504)
@@ -39,10 +39,6 @@
CSEPhase(Graph& graph)
: Phase(graph, "common subexpression elimination")
{
- for (unsigned i = 0; i < m_graph.size(); ++i)
- m_graph[i].replacement = NoNode;
-
- m_relevantToOSR.resize(m_graph.size());
}
bool run()
@@ -1023,7 +1019,7 @@
Edge edge = node.children.child(i);
if (!edge)
continue;
- if (m_relevantToOSR.get(edge.index()))
+ if (m_graph[edge].flags() & NodeRelevantToOSR)
continue;
#if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
@@ -1108,7 +1104,7 @@
}
if (node.op() == SetLocal)
- m_relevantToOSR.set(node.child1().index());
+ m_graph[node.child1()].mergeFlags(NodeRelevantToOSR);
if (!shouldGenerate)
return;
@@ -1429,20 +1425,29 @@
for (unsigned i = 0; i < LastNodeType; ++i)
m_lastSeen[i] = UINT_MAX;
- // Make all of my Phi nodes relevant to OSR.
- for (unsigned i = 0; i < block->phis.size(); ++i)
- m_relevantToOSR.set(block->phis[i]);
+ // All Phis need to already be marked as relevant to OSR, and have their
+ // replacements cleared, so we don't get confused while doing substitutions on
+ // GetLocal's.
+ for (unsigned i = 0; i < block->phis.size(); ++i) {
+ ASSERT(m_graph[block->phis[i]].flags() & NodeRelevantToOSR);
+ m_graph[block->phis[i]].replacement = NoNode;
+ }
- // Make all of my SetLocal and GetLocal nodes relevant to OSR.
+ // Make all of my SetLocal and GetLocal nodes relevant to OSR, and do some other
+ // necessary bookkeeping.
for (unsigned i = 0; i < block->size(); ++i) {
NodeIndex nodeIndex = block->at(i);
Node& node = m_graph[nodeIndex];
+
+ node.replacement = NoNode;
+
switch (node.op()) {
case SetLocal:
case GetLocal: // FIXME: The GetLocal case is only necessary until we do https://bugs.webkit.org/show_bug.cgi?id=106707.
- m_relevantToOSR.set(nodeIndex);
+ node.mergeFlags(NodeRelevantToOSR);
break;
default:
+ node.clearFlags(NodeRelevantToOSR);
break;
}
}
@@ -1457,7 +1462,6 @@
NodeIndex m_compileIndex;
unsigned m_indexInBlock;
FixedArray<unsigned, LastNodeType> m_lastSeen;
- FastBitVector m_relevantToOSR;
bool m_changed; // Only tracks changes that have a substantive effect on other optimizations.
};
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h (140503 => 140504)
--- trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h 2013-01-23 05:14:42 UTC (rev 140503)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h 2013-01-23 05:35:23 UTC (rev 140504)
@@ -36,33 +36,35 @@
// Entries in the NodeType enum (below) are composed of an id, a result type (possibly none)
// and some additional informative flags (must generate, is constant, etc).
-#define NodeResultMask 0xF
+#define NodeResultMask 0x7
#define NodeResultJS 0x1
#define NodeResultNumber 0x2
#define NodeResultInt32 0x3
#define NodeResultBoolean 0x4
#define NodeResultStorage 0x5
-#define NodeMustGenerate 0x10 // set on nodes that have side effects, and may not trivially be removed by DCE.
-#define NodeHasVarArgs 0x20
-#define NodeClobbersWorld 0x40
-#define NodeMightClobber 0x80
+#define NodeMustGenerate 0x08 // set on nodes that have side effects, and may not trivially be removed by DCE.
+#define NodeHasVarArgs 0x10
+#define NodeClobbersWorld 0x20
+#define NodeMightClobber 0x40
-#define NodeBehaviorMask 0x300
-#define NodeMayOverflow 0x100
-#define NodeMayNegZero 0x200
+#define NodeBehaviorMask 0x180
+#define NodeMayOverflow 0x080
+#define NodeMayNegZero 0x100
-#define NodeBackPropMask 0x7C00
+#define NodeBackPropMask 0x3E00
#define NodeUseBottom 0x0000
-#define NodeUsedAsNumber 0x400 // The result of this computation may be used in a context that observes fractional, or bigger-than-int32, results.
-#define NodeNeedsNegZero 0x800 // The result of this computation may be used in a context that observes -0.
-#define NodeUsedAsOther 0x1000 // The result of this computation may be used in a context that distinguishes between NaN and other things (like undefined).
+#define NodeUsedAsNumber 0x0200 // The result of this computation may be used in a context that observes fractional, or bigger-than-int32, results.
+#define NodeNeedsNegZero 0x0400 // The result of this computation may be used in a context that observes -0.
+#define NodeUsedAsOther 0x0800 // The result of this computation may be used in a context that distinguishes between NaN and other things (like undefined).
#define NodeUsedAsValue (NodeUsedAsNumber | NodeNeedsNegZero | NodeUsedAsOther)
-#define NodeUsedAsInt 0x2000 // The result of this computation is known to be used in a context that prefers, but does not require, integer values.
-#define NodeUsedAsIntLocally 0x4000 // Same as NodeUsedAsInt, but within the same basic block.
+#define NodeUsedAsInt 0x1000 // The result of this computation is known to be used in a context that prefers, but does not require, integer values.
+#define NodeUsedAsIntLocally 0x2000 // Same as NodeUsedAsInt, but within the same basic block.
-#define NodeDoesNotExit 0x8000 // This flag is negated to make it natural for the default to be that a node does exit.
+#define NodeDoesNotExit 0x4000 // This flag is negated to make it natural for the default to be that a node does exit.
+#define NodeRelevantToOSR 0x8000
+
typedef uint16_t NodeFlags;
static inline bool nodeUsedAsNumber(NodeFlags flags)
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (140503 => 140504)
--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2013-01-23 05:14:42 UTC (rev 140503)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2013-01-23 05:35:23 UTC (rev 140504)
@@ -60,8 +60,8 @@
macro(GetLocal, NodeResultJS) \
macro(SetLocal, 0) \
macro(Phantom, NodeMustGenerate | NodeDoesNotExit) \
- macro(Nop, 0 | NodeDoesNotExit) \
- macro(Phi, 0 | NodeDoesNotExit) \
+ macro(Nop, NodeDoesNotExit) \
+ macro(Phi, NodeDoesNotExit | NodeRelevantToOSR) \
macro(Flush, NodeMustGenerate | NodeDoesNotExit) \
\
/* Get the value of a local variable, without linking into the VariableAccessData */\
