MovHint should be a strong use
https://bugs.webkit.org/show_bug.cgi?id=143734
Reviewed by Geoffrey Garen.
This disables any DCE that assumes equivalence between DFG IR uses and bytecode uses. Doing
so is a major step towards allowing more fancy DFG transformations and also probably fixing
some bugs.
Just making MovHint a strong use would also completely disable DCE. So we mitigate this by
introducing a MovHint removal phase that runs in FTL.
This is a slight slowdown on Octane/gbemu, but it's basically neutral on suite averages.
* CMakeLists.txt:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
* JavaScriptCore.xcodeproj/project.pbxproj:
* bytecode/CodeOrigin.cpp:
(JSC::InlineCallFrame::dumpInContext):
* dfg/DFGDCEPhase.cpp:
(JSC::DFG::DCEPhase::fixupBlock):
* dfg/DFGDisassembler.cpp:
(JSC::DFG::Disassembler::createDumpList):
* dfg/DFGEpoch.cpp: Added.
(JSC::DFG::Epoch::dump):
* dfg/DFGEpoch.h: Added.
(JSC::DFG::Epoch::Epoch):
(JSC::DFG::Epoch::first):
(JSC::DFG::Epoch::operator!):
(JSC::DFG::Epoch::next):
(JSC::DFG::Epoch::bump):
(JSC::DFG::Epoch::operator==):
(JSC::DFG::Epoch::operator!=):
* dfg/DFGMayExit.cpp:
(JSC::DFG::mayExit):
* dfg/DFGMovHintRemovalPhase.cpp: Added.
(JSC::DFG::performMovHintRemoval):
* dfg/DFGMovHintRemovalPhase.h: Added.
* dfg/DFGNodeType.h:
* dfg/DFGPlan.cpp:
(JSC::DFG::Plan::compileInThreadImpl):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCurrentBlock):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* runtime/Options.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@183072 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp b/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp
index 36042d9..d1e3e58 100644
--- a/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp
@@ -143,23 +143,11 @@
continue;
switch (node->op()) {
- case MovHint: {
- // Check if the child is dead. MovHint's child would only be a Phantom or
- // Check if we had just killed it.
- if (node->child1()->op() == Phantom || node->child1()->op() == Check) {
- node->setOpAndDefaultFlags(ZombieHint);
- node->child1() = Edge();
- break;
- }
- break;
- }
-
- case ZombieHint: {
- // Currently we assume that DCE runs only once.
+ case MovHint:
+ case ZombieHint:
+ // These are not killable. (They once were.)
RELEASE_ASSERT_NOT_REACHED();
- break;
- }
-
+
default: {
if (node->flags() & NodeHasVarArgs) {
for (unsigned childIdx = node->firstChild(); childIdx < node->firstChild() + node->numChildren(); childIdx++) {
diff --git a/Source/JavaScriptCore/dfg/DFGDisassembler.cpp b/Source/JavaScriptCore/dfg/DFGDisassembler.cpp
index 3176100..e221c4c 100644
--- a/Source/JavaScriptCore/dfg/DFGDisassembler.cpp
+++ b/Source/JavaScriptCore/dfg/DFGDisassembler.cpp
@@ -112,8 +112,6 @@
append(result, out, previousOrigin);
Node* lastNodeForDisassembly = block->at(0);
for (size_t i = 0; i < block->size(); ++i) {
- if (!block->at(i)->willHaveCodeGenOrOSR() && !Options::showAllDFGNodes())
- continue;
MacroAssembler::Label currentLabel;
HashMap<Node*, MacroAssembler::Label>::iterator iter = m_labelForNode.find(block->at(i));
if (iter != m_labelForNode.end())
diff --git a/Source/JavaScriptCore/dfg/DFGEpoch.cpp b/Source/JavaScriptCore/dfg/DFGEpoch.cpp
new file mode 100644
index 0000000..7da1deb
--- /dev/null
+++ b/Source/JavaScriptCore/dfg/DFGEpoch.cpp
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "DFGEpoch.h"
+
+#if ENABLE(DFG_JIT)
+
+namespace JSC { namespace DFG {
+
+void Epoch::dump(PrintStream& out) const
+{
+ if (!*this)
+ out.print("none");
+ else
+ out.print(m_epoch);
+}
+
+} } // namespace JSC::DFG
+
+#endif // ENABLE(DFG_JIT)
diff --git a/Source/JavaScriptCore/dfg/DFGEpoch.h b/Source/JavaScriptCore/dfg/DFGEpoch.h
new file mode 100644
index 0000000..adb0955
--- /dev/null
+++ b/Source/JavaScriptCore/dfg/DFGEpoch.h
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef DFGEpoch_h
+#define DFGEpoch_h
+
+#if ENABLE(DFG_JIT)
+
+#include <wtf/PrintStream.h>
+
+namespace JSC { namespace DFG {
+
+// Utility class for epoch-based analyses.
+
+class Epoch {
+public:
+ Epoch()
+ : m_epoch(s_none)
+ {
+ }
+
+ static Epoch first()
+ {
+ Epoch result;
+ result.m_epoch = s_first;
+ return result;
+ }
+
+ bool operator!() const
+ {
+ return m_epoch == s_none;
+ }
+
+ Epoch next() const
+ {
+ Epoch result;
+ result.m_epoch = m_epoch + 1;
+ return result;
+ }
+
+ void bump()
+ {
+ *this = next();
+ }
+
+ bool operator==(const Epoch& other) const
+ {
+ return m_epoch == other.m_epoch;
+ }
+
+ bool operator!=(const Epoch& other) const
+ {
+ return !(*this == other);
+ }
+
+ void dump(PrintStream&) const;
+
+private:
+ static const unsigned s_none = 0;
+ static const unsigned s_first = 1;
+
+ unsigned m_epoch;
+};
+
+} } // namespace JSC::DFG
+
+#endif // ENABLE(DFG_JIT)
+
+#endif // DFGEpoch_h
+
diff --git a/Source/JavaScriptCore/dfg/DFGMayExit.cpp b/Source/JavaScriptCore/dfg/DFGMayExit.cpp
index 9670540..f2a1cc0 100644
--- a/Source/JavaScriptCore/dfg/DFGMayExit.cpp
+++ b/Source/JavaScriptCore/dfg/DFGMayExit.cpp
@@ -59,6 +59,9 @@
bool mayExit(Graph& graph, Node* node)
{
switch (node->op()) {
+ // This is a carefully curated list of nodes that definitely do not exit. We try to be very
+ // conservative when maintaining this list, because adding new node types to it doesn't
+ // generally make things a lot better but it might introduce insanely subtle bugs.
case SetArgument:
case JSConstant:
case DoubleConstant:
diff --git a/Source/JavaScriptCore/dfg/DFGMovHintRemovalPhase.cpp b/Source/JavaScriptCore/dfg/DFGMovHintRemovalPhase.cpp
new file mode 100644
index 0000000..a466459
--- /dev/null
+++ b/Source/JavaScriptCore/dfg/DFGMovHintRemovalPhase.cpp
@@ -0,0 +1,147 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "DFGMovHintRemovalPhase.h"
+
+#if ENABLE(DFG_JIT)
+
+#include "BytecodeLivenessAnalysisInlines.h"
+#include "DFGEpoch.h"
+#include "DFGForAllKills.h"
+#include "DFGGraph.h"
+#include "DFGInsertionSet.h"
+#include "DFGMayExit.h"
+#include "DFGPhase.h"
+#include "JSCInlines.h"
+#include "OperandsInlines.h"
+
+namespace JSC { namespace DFG {
+
+namespace {
+
+bool verbose = false;
+
+class MovHintRemovalPhase : public Phase {
+public:
+ MovHintRemovalPhase(Graph& graph)
+ : Phase(graph, "MovHint removal")
+ , m_state(OperandsLike, graph.block(0)->variablesAtHead)
+ , m_changed(false)
+ {
+ }
+
+ bool run()
+ {
+ if (verbose) {
+ dataLog("Graph before MovHint removal:\n");
+ m_graph.dump();
+ }
+
+ for (BasicBlock* block : m_graph.blocksInNaturalOrder())
+ handleBlock(block);
+
+ return m_changed;
+ }
+
+private:
+ void handleBlock(BasicBlock* block)
+ {
+ if (verbose)
+ dataLog("Handing block ", pointerDump(block), "\n");
+
+ // A MovHint is unnecessary if the local dies before it is used. We answer this question by
+ // maintaining the current exit epoch, and associating an epoch with each local. When a
+ // local dies, it gets the current exit epoch. If a MovHint occurs in the same epoch as its
+ // local, then it means there was no exit between the local's death and the MovHint - i.e.
+ // the MovHint is unnecessary.
+
+ Epoch currentEpoch = Epoch::first();
+
+ for (unsigned i = m_state.size(); i--;) {
+ VirtualRegister reg = m_state.virtualRegisterForIndex(i);
+ if (m_graph.isLiveInBytecode(reg, block->last()->origin.forExit))
+ m_state[i] = currentEpoch;
+ else
+ m_state[i] = Epoch();
+ }
+
+ if (verbose)
+ dataLog(" Locals: ", m_state, "\n");
+
+ // Assume that blocks after us exit.
+ currentEpoch.bump();
+
+ for (unsigned nodeIndex = block->size(); nodeIndex--;) {
+ Node* node = block->at(nodeIndex);
+
+ if (node->op() == MovHint) {
+ Epoch localEpoch = m_state.operand(node->unlinkedLocal());
+ if (verbose)
+ dataLog(" At ", node, ": current = ", currentEpoch, ", local = ", localEpoch, "\n");
+ if (!localEpoch || localEpoch == currentEpoch) {
+ node->setOpAndDefaultFlags(ZombieHint);
+ node->child1() = Edge();
+ m_changed = true;
+ }
+ m_state.operand(node->unlinkedLocal()) = Epoch();
+ }
+
+ if (mayExit(m_graph, node))
+ currentEpoch.bump();
+
+ if (nodeIndex) {
+ forAllKilledOperands(
+ m_graph, block->at(nodeIndex - 1)->origin.forExit, node,
+ [&] (VirtualRegister reg) {
+ // This function is a bit sloppy - it might claim to kill a local even if
+ // it's still live after. We need to protect against that.
+ if (!!m_state.operand(reg))
+ return;
+
+ if (verbose)
+ dataLog(" Killed operand at ", node, ": ", reg, "\n");
+ m_state.operand(reg) = currentEpoch;
+ });
+ }
+ }
+ }
+
+ Operands<Epoch> m_state;
+ bool m_changed;
+};
+
+} // anonymous namespace
+
+bool performMovHintRemoval(Graph& graph)
+{
+ SamplingRegion samplingRegion("DFG MovHint Removal Phase");
+ return runPhase<MovHintRemovalPhase>(graph);
+}
+
+} } // namespace JSC::DFG
+
+#endif // ENABLE(DFG_JIT)
+
diff --git a/Source/JavaScriptCore/dfg/DFGMovHintRemovalPhase.h b/Source/JavaScriptCore/dfg/DFGMovHintRemovalPhase.h
new file mode 100644
index 0000000..dd4c206
--- /dev/null
+++ b/Source/JavaScriptCore/dfg/DFGMovHintRemovalPhase.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef DFGMovHintRemovalPhase_h
+#define DFGMovHintRemovalPhase_h
+
+#if ENABLE(DFG_JIT)
+
+namespace JSC { namespace DFG {
+
+class Graph;
+
+// Cleans up unnecessary MovHints. A MovHint is necessary if the variable dies before there is an
+// exit.
+
+bool performMovHintRemoval(Graph&);
+
+} } // namespace JSC::DFG
+
+#endif // ENABLE(DFG_JIT)
+
+#endif // DFGMovHintRemovalPhase_h
diff --git a/Source/JavaScriptCore/dfg/DFGNodeType.h b/Source/JavaScriptCore/dfg/DFGNodeType.h
index 84f5ce3..d304ad5 100644
--- a/Source/JavaScriptCore/dfg/DFGNodeType.h
+++ b/Source/JavaScriptCore/dfg/DFGNodeType.h
@@ -63,8 +63,8 @@
macro(KillStack, NodeMustGenerate) \
macro(GetStack, NodeResultJS) \
\
- macro(MovHint, 0) \
- macro(ZombieHint, 0) \
+ macro(MovHint, NodeMustGenerate) \
+ macro(ZombieHint, NodeMustGenerate) \
macro(Phantom, NodeMustGenerate) \
macro(HardPhantom, NodeMustGenerate) /* Like Phantom, but we never remove any of its children. */ \
macro(Check, NodeMustGenerate) /* Used if we want just a type check but not liveness. Non-checking uses will be removed. */\
diff --git a/Source/JavaScriptCore/dfg/DFGPlan.cpp b/Source/JavaScriptCore/dfg/DFGPlan.cpp
index a54d6ed..f51bee0 100644
--- a/Source/JavaScriptCore/dfg/DFGPlan.cpp
+++ b/Source/JavaScriptCore/dfg/DFGPlan.cpp
@@ -47,6 +47,7 @@
#include "DFGLICMPhase.h"
#include "DFGLivenessAnalysisPhase.h"
#include "DFGLoopPreHeaderCreationPhase.h"
+#include "DFGMovHintRemovalPhase.h"
#include "DFGOSRAvailabilityAnalysisPhase.h"
#include "DFGOSREntrypointCreationPhase.h"
#include "DFGObjectAllocationSinkingPhase.h"
@@ -392,6 +393,8 @@
performCFA(dfg);
if (Options::validateFTLOSRExitLiveness())
performResurrectionForValidation(dfg);
+ if (Options::enableMovHintRemoval())
+ performMovHintRemoval(dfg);
performDCE(dfg); // We rely on this to kill dead code that won't be recognized as dead by LLVM.
performStackLayout(dfg);
performLivenessAnalysis(dfg);
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index cead30e..1c7dde5 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -1429,31 +1429,9 @@
m_codeOriginForExitProfile = m_currentNode->origin.semantic;
m_lastGeneratedNode = m_currentNode->op();
if (!m_currentNode->shouldGenerate()) {
- switch (m_currentNode->op()) {
- case JSConstant:
+ if (belongsInMinifiedGraph(m_currentNode->op()))
m_minifiedGraph->append(MinifiedNode::fromNode(m_currentNode));
- break;
-
- case SetLocal:
- RELEASE_ASSERT_NOT_REACHED();
- break;
-
- case MovHint:
- compileMovHint(m_currentNode);
- break;
-
- case ZombieHint: {
- recordSetLocal(m_currentNode->unlinkedLocal(), VirtualRegister(), DataFormatDead);
- break;
- }
-
- default:
- if (belongsInMinifiedGraph(m_currentNode->op()))
- m_minifiedGraph->append(MinifiedNode::fromNode(m_currentNode));
- break;
- }
} else {
-
if (verboseCompilationEnabled()) {
dataLogF(
"SpeculativeJIT generating Node @%d (bc#%u) at JIT offset 0x%x",
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
index 44980b1..199e9b1 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
@@ -1921,9 +1921,15 @@
break;
}
- case MovHint:
+ case MovHint: {
+ compileMovHint(m_currentNode);
+ noResult(node);
+ break;
+ }
+
case ZombieHint: {
- DFG_CRASH(m_jit.graph(), node, "Unexpected node");
+ recordSetLocal(m_currentNode->unlinkedLocal(), VirtualRegister(), DataFormatDead);
+ noResult(node);
break;
}
