- Revision
- 170929
- Author
- fpi...@apple.com
- Date
- 2014-07-09 14:16:17 -0700 (Wed, 09 Jul 2014)
Log Message
[ftlopt] Move Flush(SetLocal) store elimination to StrengthReductionPhase
https://bugs.webkit.org/show_bug.cgi?id=134739
Reviewed by Mark Hahnenberg.
I'm going to streamline CSE around clobberize() as part of
https://bugs.webkit.org/show_bug.cgi?id=134677, and so Flush(SetLocal) store
elimination wouldn't belong in CSE anymore. It doesn't quite belong anywhere, which
means that it belongs in StrengthReductionPhase, since that's intended to be our
dumping ground.
To do this I had to add some missing smarts to clobberize(). Previously clobberize()
could play a bit loose with reads of Variables because it wasn't used for store
elimination. The main client of read() was LICM, but it would only use it to
determine hoistability and anything that did a write() was not hoistable - so, we had
benign (but still wrong) missing read() calls in places that did write()s. This fixes
a bunch of those cases.
* dfg/DFGCSEPhase.cpp:
(JSC::DFG::CSEPhase::performNodeCSE):
(JSC::DFG::CSEPhase::setLocalStoreElimination): Deleted.
* dfg/DFGClobberize.cpp:
(JSC::DFG::accessesOverlap):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize): Make clobberize() smart enough for detecting when this store elimination would be sound.
* dfg/DFGStrengthReductionPhase.cpp:
(JSC::DFG::StrengthReductionPhase::handleNode): Implement the store elimination in terms of clobberize().
Modified Paths
Diff
Modified: branches/ftlopt/Source/_javascript_Core/ChangeLog (170928 => 170929)
--- branches/ftlopt/Source/_javascript_Core/ChangeLog 2014-07-09 20:55:35 UTC (rev 170928)
+++ branches/ftlopt/Source/_javascript_Core/ChangeLog 2014-07-09 21:16:17 UTC (rev 170929)
@@ -1,5 +1,35 @@
2014-07-08 Filip Pizlo <fpi...@apple.com>
+ [ftlopt] Move Flush(SetLocal) store elimination to StrengthReductionPhase
+ https://bugs.webkit.org/show_bug.cgi?id=134739
+
+ Reviewed by Mark Hahnenberg.
+
+ I'm going to streamline CSE around clobberize() as part of
+ https://bugs.webkit.org/show_bug.cgi?id=134677, and so Flush(SetLocal) store
+ elimination wouldn't belong in CSE anymore. It doesn't quite belong anywhere, which
+ means that it belongs in StrengthReductionPhase, since that's intended to be our
+ dumping ground.
+
+ To do this I had to add some missing smarts to clobberize(). Previously clobberize()
+ could play a bit loose with reads of Variables because it wasn't used for store
+ elimination. The main client of read() was LICM, but it would only use it to
+ determine hoistability and anything that did a write() was not hoistable - so, we had
+ benign (but still wrong) missing read() calls in places that did write()s. This fixes
+ a bunch of those cases.
+
+ * dfg/DFGCSEPhase.cpp:
+ (JSC::DFG::CSEPhase::performNodeCSE):
+ (JSC::DFG::CSEPhase::setLocalStoreElimination): Deleted.
+ * dfg/DFGClobberize.cpp:
+ (JSC::DFG::accessesOverlap):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize): Make clobberize() smart enough for detecting when this store elimination would be sound.
+ * dfg/DFGStrengthReductionPhase.cpp:
+ (JSC::DFG::StrengthReductionPhase::handleNode): Implement the store elimination in terms of clobberize().
+
+2014-07-08 Filip Pizlo <fpi...@apple.com>
+
[ftlopt] Phantom simplification should be in its own phase
https://bugs.webkit.org/show_bug.cgi?id=134742
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGCSEPhase.cpp (170928 => 170929)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2014-07-09 20:55:35 UTC (rev 170928)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2014-07-09 21:16:17 UTC (rev 170929)
@@ -719,77 +719,6 @@
return false;
}
- Node* setLocalStoreElimination(VirtualRegister local)
- {
- for (unsigned i = m_indexInBlock; i--;) {
- Node* node = m_currentBlock->at(i);
- switch (node->op()) {
- case GetLocal:
- case Flush:
- if (node->local() == local)
- return nullptr;
- break;
-
- case GetLocalUnlinked:
- if (node->unlinkedLocal() == local)
- return nullptr;
- break;
-
- case SetLocal: {
- if (node->local() == local)
- return node;
- break;
- }
-
- case GetClosureVar:
- case PutClosureVar:
- if (static_cast<VirtualRegister>(node->varNumber()) == local)
- return nullptr;
- break;
-
- case GetMyScope:
- case SkipTopScope:
- if (node->origin.semantic.inlineCallFrame)
- break;
- if (m_graph.uncheckedActivationRegister() == local)
- return nullptr;
- break;
-
- case CheckArgumentsNotCreated:
- case GetMyArgumentsLength:
- case GetMyArgumentsLengthSafe:
- if (m_graph.uncheckedArgumentsRegisterFor(node->origin.semantic) == local)
- return nullptr;
- break;
-
- case GetMyArgumentByVal:
- case GetMyArgumentByValSafe:
- return nullptr;
-
- case GetByVal:
- // If this is accessing arguments then it's potentially accessing locals.
- if (node->arrayMode().type() == Array::Arguments)
- return nullptr;
- break;
-
- case CreateArguments:
- case TearOffActivation:
- case TearOffArguments:
- // If an activation is being torn off then it means that captured variables
- // are live. We could be clever here and check if the local qualifies as an
- // argument register. But that seems like it would buy us very little since
- // any kind of tear offs are rare to begin with.
- return nullptr;
-
- default:
- break;
- }
- if (m_graph.clobbersWorld(node))
- return nullptr;
- }
- return nullptr;
- }
-
bool setReplacement(Node* replacement)
{
if (!replacement)
@@ -1048,23 +977,6 @@
eliminate();
break;
- case Flush:
- // This is needed for arguments simplification to work. We need to eliminate the
- // redundancy between op_enter's undefined-all-the-things and the subsequent
- // op_init_lazy_reg.
-
- ASSERT(m_graph.m_form != SSA);
-
- if (Node* setLocal = setLocalStoreElimination(node->local())) {
- node->convertToPhantom();
- Node* dataNode = setLocal->child1().node();
- ASSERT(dataNode->hasResult());
- node->child1() = dataNode->defaultEdge();
- m_graph.dethread();
- m_changed = true;
- }
- break;
-
default:
// do nothing.
break;
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGClobberize.cpp (170928 => 170929)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGClobberize.cpp 2014-07-09 20:55:35 UTC (rev 170928)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGClobberize.cpp 2014-07-09 21:16:17 UTC (rev 170929)
@@ -40,6 +40,13 @@
return addWrite.result();
}
+bool accessesOverlap(Graph& graph, Node* node, AbstractHeap heap)
+{
+ AbstractHeapOverlaps addAccess(heap);
+ clobberize(graph, node, addAccess, addAccess);
+ return addAccess.result();
+}
+
bool writesOverlap(Graph& graph, Node* node, AbstractHeap heap)
{
NoOpClobberize addRead;
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGClobberize.h (170928 => 170929)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGClobberize.h 2014-07-09 20:55:35 UTC (rev 170928)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGClobberize.h 2014-07-09 21:16:17 UTC (rev 170929)
@@ -128,7 +128,6 @@
case ZombieHint:
case Upsilon:
case Phi:
- case Flush:
case PhantomLocal:
case SetArgument:
case PhantomArguments:
@@ -152,6 +151,11 @@
write(SideState);
return;
+ case Flush:
+ read(AbstractHeap(Variables, node->local()));
+ write(SideState);
+ return;
+
case VariableWatchpoint:
case TypedArrayWatchpoint:
read(Watchpoint_fire);
@@ -164,13 +168,20 @@
return;
case CreateActivation:
+ read(HeapObjectCount);
+ write(HeapObjectCount);
+ write(SideState);
+ write(Watchpoint_fire);
+ return;
+
case CreateArguments:
+ read(Variables);
read(HeapObjectCount);
write(HeapObjectCount);
write(SideState);
write(Watchpoint_fire);
return;
-
+
case FunctionReentryWatchpoint:
read(Watchpoint_fire);
return;
@@ -629,10 +640,12 @@
}
case TearOffActivation:
+ read(Variables);
write(JSVariableObject_registers);
return;
case TearOffArguments:
+ read(Variables);
write(Arguments_registers);
return;
@@ -714,6 +727,7 @@
bool m_result;
};
+bool accessesOverlap(Graph&, Node*, AbstractHeap);
bool writesOverlap(Graph&, Node*, AbstractHeap);
} } // namespace JSC::DFG
Modified: branches/ftlopt/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp (170928 => 170929)
--- branches/ftlopt/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp 2014-07-09 20:55:35 UTC (rev 170928)
+++ branches/ftlopt/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp 2014-07-09 21:16:17 UTC (rev 170929)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -28,6 +28,8 @@
#if ENABLE(DFG_JIT)
+#include "DFGAbstractHeap.h"
+#include "DFGClobberize.h"
#include "DFGGraph.h"
#include "DFGInsertionSet.h"
#include "DFGPhase.h"
@@ -213,6 +215,34 @@
break;
}
+ case Flush: {
+ ASSERT(m_graph.m_form != SSA);
+
+ Node* setLocal = nullptr;
+ VirtualRegister local = m_node->local();
+
+ for (unsigned i = m_nodeIndex; i--;) {
+ Node* node = m_block->at(i);
+ if (node->op() == SetLocal && node->local() == local) {
+ setLocal = node;
+ break;
+ }
+ if (accessesOverlap(m_graph, node, AbstractHeap(Variables, local)))
+ break;
+ }
+
+ if (!setLocal)
+ break;
+
+ m_node->convertToPhantom();
+ Node* dataNode = setLocal->child1().node();
+ DFG_ASSERT(m_graph, m_node, dataNode->hasResult());
+ m_node->child1() = dataNode->defaultEdge();
+ m_graph.dethread();
+ m_changed = true;
+ break;
+ }
+
default:
break;
}
