- Revision
- 170064
- Author
- [email protected]
- Date
- 2014-06-17 12:01:55 -0700 (Tue, 17 Jun 2014)
Log Message
[ftlopt] Fold constant Phis
https://bugs.webkit.org/show_bug.cgi?id=133967
Reviewed by Mark Hahnenberg.
It's surprising but we didn't really do this before. Or, rather, we only did it
incidentally when we would likely crash if it ever happened.
Making this work required cleaning up the validater a bit, so I did that too. I also added
mayExit() validation for nodes that didn't have origin.forExit (i.e. nodes that end up in
the Phi header of basic blocks). But this required beefing up mayExit() a bit.
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGAdjacencyList.h:
(JSC::DFG::AdjacencyList::isEmpty):
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::run):
(JSC::DFG::ConstantFoldingPhase::foldConstants):
(JSC::DFG::ConstantFoldingPhase::fixUpsilons):
* dfg/DFGInPlaceAbstractState.h:
* dfg/DFGLICMPhase.cpp:
(JSC::DFG::LICMPhase::run):
(JSC::DFG::LICMPhase::attemptHoist):
* dfg/DFGMayExit.cpp:
(JSC::DFG::mayExit):
* dfg/DFGValidate.cpp:
(JSC::DFG::Validate::validate):
(JSC::DFG::Validate::validateSSA):
Modified Paths
Diff
Modified: branches/ftlopt/Source/_javascript_Core/ChangeLog (170063 => 170064)
--- branches/ftlopt/Source/_javascript_Core/ChangeLog 2014-06-17 19:00:42 UTC (rev 170063)
+++ branches/ftlopt/Source/_javascript_Core/ChangeLog 2014-06-17 19:01:55 UTC (rev 170064)
@@ -1,5 +1,37 @@
2014-06-17 Filip Pizlo <[email protected]>
+ [ftlopt] Fold constant Phis
+ https://bugs.webkit.org/show_bug.cgi?id=133967
+
+ Reviewed by Mark Hahnenberg.
+
+ It's surprising but we didn't really do this before. Or, rather, we only did it
+ incidentally when we would likely crash if it ever happened.
+
+ Making this work required cleaning up the validater a bit, so I did that too. I also added
+ mayExit() validation for nodes that didn't have origin.forExit (i.e. nodes that end up in
+ the Phi header of basic blocks). But this required beefing up mayExit() a bit.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGAdjacencyList.h:
+ (JSC::DFG::AdjacencyList::isEmpty):
+ * dfg/DFGConstantFoldingPhase.cpp:
+ (JSC::DFG::ConstantFoldingPhase::run):
+ (JSC::DFG::ConstantFoldingPhase::foldConstants):
+ (JSC::DFG::ConstantFoldingPhase::fixUpsilons):
+ * dfg/DFGInPlaceAbstractState.h:
+ * dfg/DFGLICMPhase.cpp:
+ (JSC::DFG::LICMPhase::run):
+ (JSC::DFG::LICMPhase::attemptHoist):
+ * dfg/DFGMayExit.cpp:
+ (JSC::DFG::mayExit):
+ * dfg/DFGValidate.cpp:
+ (JSC::DFG::Validate::validate):
+ (JSC::DFG::Validate::validateSSA):
+
+2014-06-17 Filip Pizlo <[email protected]>
+
[ftlopt] Get rid of NodeDoesNotExit and also get rid of StoreEliminationPhase
https://bugs.webkit.org/show_bug.cgi?id=133985
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (170063 => 170064)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2014-06-17 19:00:42 UTC (rev 170063)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2014-06-17 19:01:55 UTC (rev 170064)
@@ -1759,7 +1759,10 @@
case Phi:
RELEASE_ASSERT(m_graph.m_form == SSA);
- // The state of this node would have already been decided.
+ // The state of this node would have already been decided, but it may have become a
+ // constant, in which case we'd like to know.
+ if (forNode(node).m_value)
+ m_state.setFoundConstants(true);
break;
case Upsilon: {
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGAdjacencyList.h (170063 => 170064)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGAdjacencyList.h 2014-06-17 19:00:42 UTC (rev 170063)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGAdjacencyList.h 2014-06-17 19:01:55 UTC (rev 170064)
@@ -65,6 +65,8 @@
setNumChildren(numChildren);
}
+ bool isEmpty() const { return !child1(); }
+
const Edge& child(unsigned i) const
{
ASSERT(i < Size);
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp (170063 => 170064)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp 2014-06-17 19:00:42 UTC (rev 170063)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp 2014-06-17 19:01:55 UTC (rev 170064)
@@ -62,6 +62,16 @@
changed |= foldConstants(block);
}
+ if (changed && m_graph.m_form == SSA) {
+ // It's now possible that we have Upsilons pointed at JSConstants. Fix that.
+ for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
+ BasicBlock* block = m_graph.block(blockIndex);
+ if (!block)
+ continue;
+ fixUpsilons(block);
+ }
+ }
+
return changed;
}
@@ -313,8 +323,10 @@
AdjacencyList children = node->children;
m_graph.convertToConstant(node, value);
- m_insertionSet.insertNode(
- indexInBlock, SpecNone, Phantom, origin, children);
+ if (!children.isEmpty()) {
+ m_insertionSet.insertNode(
+ indexInBlock, SpecNone, Phantom, origin, children);
+ }
changed = true;
}
@@ -465,6 +477,27 @@
OpInfo(m_graph.addStructureSet(structure)), Edge(weakConstant, CellUse));
}
+ void fixUpsilons(BasicBlock* block)
+ {
+ for (unsigned nodeIndex = block->size(); nodeIndex--;) {
+ Node* node = block->at(nodeIndex);
+ if (node->op() != Upsilon)
+ continue;
+ switch (node->phi()->op()) {
+ case Phi:
+ break;
+ case JSConstant:
+ case DoubleConstant:
+ case Int52Constant:
+ node->convertToPhantom();
+ break;
+ default:
+ DFG_CRASH(m_graph, node, "Bad Upsilon phi() pointer");
+ break;
+ }
+ }
+ }
+
InPlaceAbstractState m_state;
AbstractInterpreter<InPlaceAbstractState> m_interpreter;
InsertionSet m_insertionSet;
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.h (170063 => 170064)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.h 2014-06-17 19:00:42 UTC (rev 170063)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.h 2014-06-17 19:01:55 UTC (rev 170064)
@@ -128,6 +128,12 @@
void setStructureClobberState(StructureClobberState value) { m_structureClobberState = value; }
void setIsValid(bool isValid) { m_isValid = isValid; }
void setBranchDirection(BranchDirection branchDirection) { m_branchDirection = branchDirection; }
+
+ // This method is evil - it causes a huge maintenance headache and there is a gross amount of
+ // code devoted to it. It would be much nicer to just always run the constant folder on each
+ // block. But, the last time we did it, it was a 1% SunSpider regression:
+ // https://bugs.webkit.org/show_bug.cgi?id=133947
+ // So, we should probably keep this method.
void setFoundConstants(bool foundConstants) { m_foundConstants = foundConstants; }
private:
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGLICMPhase.cpp (170063 => 170064)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGLICMPhase.cpp 2014-06-17 19:00:42 UTC (rev 170063)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGLICMPhase.cpp 2014-06-17 19:01:55 UTC (rev 170064)
@@ -69,7 +69,7 @@
bool run()
{
- ASSERT(m_graph.m_form == SSA);
+ DFG_ASSERT(m_graph, nullptr, m_graph.m_form == SSA);
m_graph.m_dominators.computeIfNecessary(m_graph);
m_graph.m_naturalLoops.computeIfNecessary(m_graph);
@@ -124,11 +124,11 @@
BasicBlock* predecessor = header->predecessors[i];
if (m_graph.m_dominators.dominates(header, predecessor))
continue;
- RELEASE_ASSERT(!preHeader || preHeader == predecessor);
+ DFG_ASSERT(m_graph, nullptr, !preHeader || preHeader == predecessor);
preHeader = predecessor;
}
- RELEASE_ASSERT(preHeader->last()->op() == Jump);
+ DFG_ASSERT(m_graph, preHeader->last(), preHeader->last()->op() == Jump);
data.preHeader = preHeader;
}
@@ -268,7 +268,7 @@
// It just so happens that all of the nodes we currently know how to hoist
// don't have var-arg children. That may change and then we can fix this
// code. But for now we just assert that's the case.
- RELEASE_ASSERT(!(node->flags() & NodeHasVarArgs));
+ DFG_ASSERT(m_graph, node, !(node->flags() & NodeHasVarArgs));
nodeRef = m_graph.addNode(SpecNone, Phantom, originalOrigin, node->children);
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGMayExit.cpp (170063 => 170064)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGMayExit.cpp 2014-06-17 19:00:42 UTC (rev 170063)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGMayExit.cpp 2014-06-17 19:01:55 UTC (rev 170064)
@@ -70,6 +70,9 @@
case GetLocal:
case LoopHint:
case PhantomArguments:
+ case Phi:
+ case Upsilon:
+ case ZombieHint:
break;
default:
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGValidate.cpp (170063 => 170064)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGValidate.cpp 2014-06-17 19:00:42 UTC (rev 170063)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGValidate.cpp 2014-06-17 19:01:55 UTC (rev 170064)
@@ -29,6 +29,7 @@
#if ENABLE(DFG_JIT)
#include "CodeBlockWithJITType.h"
+#include "DFGMayExit.h"
#include "JSCInlines.h"
#include <wtf/Assertions.h>
#include <wtf/BitVector.h>
@@ -197,11 +198,16 @@
for (size_t i = 0; i < block->size(); ++i) {
Node* node = block->at(i);
- if (node->hasStructure())
- VALIDATE((node), !!node->structure());
+ VALIDATE((node), !mayExit(m_graph, node) || node->origin.forExit.isSet());
+ VALIDATE((node), !node->hasStructure() || !!node->structure());
+ VALIDATE((node), !node->hasFunction() || node->function()->value().isFunction());
- if (node->hasFunction())
- VALIDATE((node), node->function()->value().isFunction());
+ if (!(node->flags() & NodeHasVarArgs)) {
+ if (!node->child2())
+ VALIDATE((node), !node->child3());
+ if (!node->child1())
+ VALIDATE((node), !node->child2());
+ }
switch (node->op()) {
case MakeRope:
@@ -447,18 +453,18 @@
continue;
unsigned nodeIndex = 0;
- for (; nodeIndex < block->size() && !block->at(nodeIndex)->origin.isSet(); nodeIndex++) { }
+ 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.isSet());
+ VALIDATE((block->at(nodeIndex)), block->at(nodeIndex)->origin.forExit.isSet());
for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
Node* node = block->at(nodeIndex);
switch (node->op()) {
case Phi:
- VALIDATE((node), !node->origin.isSet());
+ VALIDATE((node), !node->origin.forExit.isSet());
break;
default:
