Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (189012 => 189013)
--- trunk/Source/_javascript_Core/ChangeLog 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-08-27 07:16:07 UTC (rev 189013)
@@ -1,3 +1,56 @@
+2015-08-26 Filip Pizlo <[email protected]>
+
+ Node::origin should always be set, and the dead zone due to SSA Phis can just use exitOK=false
+ https://bugs.webkit.org/show_bug.cgi?id=148462
+
+ Reviewed by Saam Barati.
+
+ The need to label nodes that absolutely cannot exit was first observed when we introduced SSA form.
+ We indicated this by not setting the CodeOrigin.
+
+ But just recently (http://trac.webkit.org/changeset/188979), we added a more comprehensive "exitOK"
+ bit in NodeOrigin. After that change, there were two ways of indicating that you cannot exit:
+ !exitOK and an unset NodeOrigin. An unset NodeOrigin implied !exitOK.
+
+ Now, this change is about removing the old way so that we only use !exitOK. From now on, all nodes
+ must have their NodeOrigin set, and the IR validation will check this. This means that I could
+ remove various pieces of cruft for dealing with unset NodeOrigins, but I did have to add some new
+ cruft to ensure that all nodes we create have a NodeOrigin.
+
+ This change simplifies our IR by having a simpler rule about when NodeOrigin is set: it's always
+ set.
+
+ * dfg/DFGBasicBlock.cpp:
+ (JSC::DFG::BasicBlock::isInBlock):
+ (JSC::DFG::BasicBlock::removePredecessor):
+ (JSC::DFG::BasicBlock::firstOriginNode): Deleted.
+ (JSC::DFG::BasicBlock::firstOrigin): Deleted.
+ * dfg/DFGBasicBlock.h:
+ (JSC::DFG::BasicBlock::begin):
+ (JSC::DFG::BasicBlock::end):
+ (JSC::DFG::BasicBlock::numSuccessors):
+ (JSC::DFG::BasicBlock::successor):
+ * dfg/DFGCombinedLiveness.cpp:
+ (JSC::DFG::liveNodesAtHead):
+ * dfg/DFGConstantHoistingPhase.cpp:
+ * dfg/DFGCriticalEdgeBreakingPhase.cpp:
+ (JSC::DFG::CriticalEdgeBreakingPhase::breakCriticalEdge):
+ * dfg/DFGForAllKills.h:
+ (JSC::DFG::forAllKilledOperands):
+ * dfg/DFGIntegerRangeOptimizationPhase.cpp:
+ * dfg/DFGLoopPreHeaderCreationPhase.cpp:
+ (JSC::DFG::createPreHeader):
+ (JSC::DFG::LoopPreHeaderCreationPhase::run):
+ * dfg/DFGOSRAvailabilityAnalysisPhase.cpp:
+ (JSC::DFG::OSRAvailabilityAnalysisPhase::run):
+ * dfg/DFGObjectAllocationSinkingPhase.cpp:
+ * dfg/DFGPutStackSinkingPhase.cpp:
+ * dfg/DFGSSAConversionPhase.cpp:
+ (JSC::DFG::SSAConversionPhase::run):
+ * dfg/DFGValidate.cpp:
+ (JSC::DFG::Validate::validate):
+ (JSC::DFG::Validate::validateSSA):
+
2015-08-26 Saam barati <[email protected]>
MarkedBlock::allocateBlock will have the wrong allocation size when (sizeof(MarkedBlock) + bytes) is divisible by WTF::pageSize()
Modified: trunk/Source/_javascript_Core/dfg/DFGBasicBlock.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGBasicBlock.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGBasicBlock.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -102,21 +102,6 @@
return false;
}
-Node* BasicBlock::firstOriginNode()
-{
- for (Node* node : *this) {
- if (node->origin.isSet())
- return node;
- }
- RELEASE_ASSERT_NOT_REACHED();
- return nullptr;
-}
-
-NodeOrigin BasicBlock::firstOrigin()
-{
- return firstOriginNode()->origin;
-}
-
void BasicBlock::removePredecessor(BasicBlock* block)
{
for (unsigned i = 0; i < predecessors.size(); ++i) {
Modified: trunk/Source/_javascript_Core/dfg/DFGBasicBlock.h (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGBasicBlock.h 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGBasicBlock.h 2015-08-27 07:16:07 UTC (rev 189013)
@@ -141,10 +141,7 @@
BlockNodeList::iterator begin() { return m_nodes.begin(); }
BlockNodeList::iterator end() { return m_nodes.end(); }
-
- Node* firstOriginNode();
- NodeOrigin firstOrigin();
-
+
unsigned numSuccessors() { return terminal()->numSuccessors(); }
BasicBlock*& successor(unsigned index)
Modified: trunk/Source/_javascript_Core/dfg/DFGCombinedLiveness.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGCombinedLiveness.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGCombinedLiveness.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -43,7 +43,7 @@
AvailabilityMap& availabilityMap = block->ssa->availabilityAtHead;
graph.forAllLocalsLiveInBytecode(
- block->firstOrigin().forExit,
+ block->at(0)->origin.forExit,
[&] (VirtualRegister reg) {
availabilityMap.closeStartingWithLocal(
reg,
Modified: trunk/Source/_javascript_Core/dfg/DFGConstantHoistingPhase.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGConstantHoistingPhase.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGConstantHoistingPhase.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -94,7 +94,7 @@
HashMap<FrozenValue*, Node*>& values = valuesFor(node->op());
auto result = values.add(node->constant(), node);
if (result.isNewEntry)
- node->origin = NodeOrigin();
+ node->origin = m_graph.block(0)->at(0)->origin;
else {
node->setReplacement(result.iterator->value);
toFree.append(node);
Modified: trunk/Source/_javascript_Core/dfg/DFGCriticalEdgeBreakingPhase.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGCriticalEdgeBreakingPhase.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGCriticalEdgeBreakingPhase.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -77,7 +77,7 @@
// don't know its execution frequency.
BasicBlock* pad = m_insertionSet.insertBefore(*successor, PNaN);
pad->appendNode(
- m_graph, SpecNone, Jump, (*successor)->firstOrigin(), OpInfo(*successor));
+ m_graph, SpecNone, Jump, (*successor)->at(0)->origin, OpInfo(*successor));
pad->predecessors.append(predecessor);
(*successor)->replacePredecessor(predecessor, pad);
Modified: trunk/Source/_javascript_Core/dfg/DFGForAllKills.h (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGForAllKills.h 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGForAllKills.h 2015-08-27 07:16:07 UTC (rev 189013)
@@ -55,33 +55,18 @@
CodeOrigin after = nodeAfter->origin.forExit;
VirtualRegister alreadyNoted;
- if (!!after) {
- // If we MovHint something that is live at the time, then we kill the old value.
- if (nodeAfter->containsMovHint()) {
- VirtualRegister reg = nodeAfter->unlinkedLocal();
- if (graph.isLiveInBytecode(reg, after)) {
- functor(reg);
- alreadyNoted = reg;
- }
+ // If we MovHint something that is live at the time, then we kill the old value.
+ if (nodeAfter->containsMovHint()) {
+ VirtualRegister reg = nodeAfter->unlinkedLocal();
+ if (graph.isLiveInBytecode(reg, after)) {
+ functor(reg);
+ alreadyNoted = reg;
}
}
- if (!before) {
- if (!after)
- return;
- // The true before-origin is the origin at predecessors that jump to us. But there can be
- // many such predecessors and they will likely all have a different origin. So, it's better
- // to do the conservative thing.
- graph.forAllLocalsLiveInBytecode(after, functor);
- return;
- }
-
if (before == after)
return;
- // before could be unset even if after is, but the opposite cannot happen.
- ASSERT(!!after);
-
// It's easier to do this if the inline call frames are the same. This is way faster than the
// other loop, below.
if (before.inlineCallFrame == after.inlineCallFrame) {
Modified: trunk/Source/_javascript_Core/dfg/DFGIntegerRangeOptimizationPhase.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGIntegerRangeOptimizationPhase.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGIntegerRangeOptimizationPhase.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -999,7 +999,7 @@
}
}
if (!m_zero) {
- m_zero = m_insertionSet.insertConstant(0, NodeOrigin(), jsNumber(0));
+ m_zero = m_insertionSet.insertConstant(0, m_graph.block(0)->at(0)->origin, jsNumber(0));
m_insertionSet.execute(m_graph.block(0));
}
Modified: trunk/Source/_javascript_Core/dfg/DFGLoopPreHeaderCreationPhase.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGLoopPreHeaderCreationPhase.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGLoopPreHeaderCreationPhase.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -42,7 +42,7 @@
// Don't bother to preserve execution frequencies for now.
BasicBlock* preHeader = insertionSet.insertBefore(block, PNaN);
preHeader->appendNode(
- graph, SpecNone, Jump, block->firstOrigin(), OpInfo(block));
+ graph, SpecNone, Jump, block->at(0)->origin, OpInfo(block));
for (unsigned predecessorIndex = 0; predecessorIndex < block->predecessors.size(); predecessorIndex++) {
BasicBlock* predecessor = block->predecessors[predecessorIndex];
@@ -108,7 +108,7 @@
// A pre-header is most useful if it's possible to exit from its terminal. Hence
// if the terminal of the existing pre-header doesn't allow for exit, but the first
// origin of the loop header does, then we should create a new pre-header.
- if (!needsNewPreHeader && loop.header()->firstOrigin().exitOK
+ if (!needsNewPreHeader && loop.header()->at(0)->origin.exitOK
&& !existingPreHeader->terminal()->origin.exitOK)
needsNewPreHeader = true;
Modified: trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -91,7 +91,7 @@
BasicBlock* successor = block->successor(successorIndex);
successor->ssa->availabilityAtHead.merge(calculator.m_availability);
successor->ssa->availabilityAtHead.pruneByLiveness(
- m_graph, successor->firstOrigin().forExit);
+ m_graph, successor->at(0)->origin.forExit);
}
}
} while (changed);
Modified: trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -1548,7 +1548,7 @@
// with useless constants everywhere
HashMap<FrozenValue*, Node*> lazyMapping;
if (!m_bottom)
- m_bottom = m_insertionSet.insertConstant(0, NodeOrigin(), jsNumber(1927));
+ m_bottom = m_insertionSet.insertConstant(0, m_graph.block(0)->at(0)->origin, jsNumber(1927));
for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
m_heap = m_heapAtHead[block];
@@ -1621,7 +1621,7 @@
if (m_heapAtHead[block].follow(location))
return nullptr;
- Node* phiNode = m_graph.addNode(SpecHeapTop, Phi, NodeOrigin());
+ Node* phiNode = m_graph.addNode(SpecHeapTop, Phi, block->at(0)->origin.withInvalidExit());
phiNode->mergeFlags(NodeResultJS);
return phiNode;
});
@@ -1638,7 +1638,7 @@
if (!m_heapAtHead[block].getAllocation(identifier).isEscapedAllocation())
return nullptr;
- Node* phiNode = m_graph.addNode(SpecHeapTop, Phi, NodeOrigin());
+ Node* phiNode = m_graph.addNode(SpecHeapTop, Phi, block->at(0)->origin.withInvalidExit());
phiNode->mergeFlags(NodeResultJS);
return phiNode;
});
@@ -1669,7 +1669,9 @@
if (m_sinkCandidates.contains(location.base())) {
m_insertionSet.insert(
- 0, location.createHint(m_graph, NodeOrigin(), phiDef->value()));
+ 0,
+ location.createHint(
+ m_graph, block->at(0)->origin.withInvalidExit(), phiDef->value()));
}
}
@@ -1680,7 +1682,9 @@
Node* identifier = indexToNode[variable->index()];
m_escapeeToMaterialization.add(identifier, phiDef->value());
bool canExit = false;
- insertOSRHintsForUpdate(0, NodeOrigin(), canExit, availabilityCalculator.m_availability, identifier, phiDef->value());
+ insertOSRHintsForUpdate(
+ 0, block->at(0)->origin, canExit,
+ availabilityCalculator.m_availability, identifier, phiDef->value());
}
if (verbose) {
Modified: trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -358,7 +358,7 @@
if (verbose)
dataLog("Adding Phi for ", operand, " at ", pointerDump(block), "\n");
- Node* phiNode = m_graph.addNode(SpecHeapTop, Phi, NodeOrigin());
+ Node* phiNode = m_graph.addNode(SpecHeapTop, Phi, block->at(0)->origin.withInvalidExit());
phiNode->mergeFlags(resultFor(format));
return phiNode;
});
Modified: trunk/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -149,7 +149,7 @@
return nullptr;
Node* phiNode = m_graph.addNode(
- variable->prediction(), Phi, NodeOrigin());
+ variable->prediction(), Phi, block->at(0)->origin.withInvalidExit());
FlushFormat format = variable->flushFormat();
NodeFlags result = resultFor(format);
phiNode->mergeFlags(result);
@@ -252,9 +252,12 @@
valueForOperand.operand(variable->local()) = phiDef->value();
m_insertionSet.insertNode(
- phiInsertionPoint, SpecNone, MovHint, NodeOrigin(),
+ phiInsertionPoint, SpecNone, MovHint, block->at(0)->origin.withInvalidExit(),
OpInfo(variable->local().offset()), phiDef->value()->defaultEdge());
}
+
+ if (block->at(0)->origin.exitOK)
+ m_insertionSet.insertNode(phiInsertionPoint, SpecNone, ExitOK, block->at(0)->origin);
for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
Node* node = block->at(nodeIndex);
Modified: trunk/Source/_javascript_Core/dfg/DFGValidate.cpp (189012 => 189013)
--- trunk/Source/_javascript_Core/dfg/DFGValidate.cpp 2015-08-27 05:54:21 UTC (rev 189012)
+++ trunk/Source/_javascript_Core/dfg/DFGValidate.cpp 2015-08-27 07:16:07 UTC (rev 189013)
@@ -187,6 +187,7 @@
for (size_t i = 0; i < block->size(); ++i) {
Node* node = block->at(i);
+ VALIDATE((node), node->origin.isSet());
VALIDATE((node), node->origin.semantic.isSet() == node->origin.forExit.isSet());
VALIDATE((node), !(!node->origin.forExit.isSet() && node->origin.exitOK));
VALIDATE((node), !(mayExit(m_graph, node) == Exits && !node->origin.exitOK));
@@ -526,20 +527,21 @@
continue;
VALIDATE((block), block->phis.isEmpty());
+
+ bool didSeeExitOK = false;
- unsigned nodeIndex = 0;
- for (; nodeIndex < block->size() && !block->at(nodeIndex)->origin.forExit.isSet(); nodeIndex++) { }
-
- VALIDATE((block), nodeIndex < block->size());
-
- for (; nodeIndex < block->size(); nodeIndex++)
- VALIDATE((block->at(nodeIndex)), block->at(nodeIndex)->origin.forExit.isSet());
-
for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
Node* node = block->at(nodeIndex);
+ didSeeExitOK |= node->origin.exitOK;
switch (node->op()) {
case Phi:
- VALIDATE((node), !node->origin.forExit.isSet());
+ // Phi cannot exit, and it would be wrong to hoist anything to the Phi that could
+ // exit.
+ VALIDATE((node), !node->origin.exitOK);
+
+ // It never makes sense to have exitOK anywhere in the block before a Phi. It's only
+ // OK to exit after all Phis are done.
+ VALIDATE((node), !didSeeExitOK);
break;
case GetLocal:
