Changeset 183072 in webkit

Timestamp:

Apr 21, 2015 1:16:18 PM (9 years ago)

Author:

fpizlo@apple.com

Message:

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:

Location:

trunk/Source/JavaScriptCore

Files:

• CMakeLists.txt (modified) (2 diffs)
• ChangeLog (modified) (1 diff)
• JavaScriptCore.vcxproj/JavaScriptCore.vcxproj (modified) (4 diffs)
• JavaScriptCore.xcodeproj/project.pbxproj (modified) (8 diffs)
• bytecode/CodeOrigin.cpp (modified) (1 diff)
• dfg/DFGDCEPhase.cpp (modified) (1 diff)
• dfg/DFGDisassembler.cpp (modified) (1 diff)
• dfg/DFGEpoch.cpp (added)
• dfg/DFGEpoch.h (added)
• dfg/DFGMayExit.cpp (modified) (1 diff)
• dfg/DFGMovHintRemovalPhase.cpp (added)
• dfg/DFGMovHintRemovalPhase.h (added)
• dfg/DFGNodeType.h (modified) (1 diff)
• dfg/DFGPlan.cpp (modified) (2 diffs)
• dfg/DFGSpeculativeJIT.cpp (modified) (1 diff)
• dfg/DFGSpeculativeJIT64.cpp (modified) (1 diff)
• runtime/Options.h (modified) (1 diff)

trunk/Source/JavaScriptCore/CMakeLists.txt

@@ -157 +157 @@
     dfg/DFGDriver.cpp
     dfg/DFGEdge.cpp
+    dfg/DFGEpoch.cpp
     dfg/DFGFailedFinalizer.cpp
     dfg/DFGFinalizer.cpp
@@ -182 +183 @@
     dfg/DFGMayExit.cpp
     dfg/DFGMinifiedNode.cpp
+    dfg/DFGMovHintRemovalPhase.cpp
     dfg/DFGNaiveDominators.cpp
     dfg/DFGNaturalLoops.cpp

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-04-19  Filip Pizlo  <fpizlo@apple.com>
+
+        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:
+
 2015-04-21  Basile Clement  <basile_clement@apple.com>
 

trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj

@@ -401 +401 @@
     <ClCompile Include="..\dfg\DFGDriver.cpp" />
     <ClCompile Include="..\dfg\DFGEdge.cpp" />
+    <ClCompile Include="..\dfg\DFGEpoch.cpp" />
     <ClCompile Include="..\dfg\DFGFailedFinalizer.cpp" />
     <ClCompile Include="..\dfg\DFGFinalizer.cpp" />
@@ -426 +427 @@
     <ClCompile Include="..\dfg\DFGMayExit.cpp" />
     <ClCompile Include="..\dfg\DFGMinifiedNode.cpp" />
+    <ClCompile Include="..\dfg\DFGMovHintRemovalPhase.cpp" />
     <ClCompile Include="..\dfg\DFGNaiveDominators.cpp" />
     <ClCompile Include="..\dfg\DFGNaturalLoops.cpp" />
@@ -1082 +1084 @@
     <ClInclude Include="..\dfg\DFGEdgeDominates.h" />
     <ClInclude Include="..\dfg\DFGEdgeUsesStructure.h" />
+    <ClInclude Include="..\dfg\DFGEpoch.h" />
     <ClInclude Include="..\dfg\DFGFailedFinalizer.h" />
     <ClInclude Include="..\dfg\DFGFiltrationResult.h" />
@@ -1116 +1119 @@
     <ClInclude Include="..\dfg\DFGMinifiedID.h" />
     <ClInclude Include="..\dfg\DFGMinifiedNode.h" />
+    <ClInclude Include="..\dfg\DFGMovHintRemovalPhase.h" />
     <ClInclude Include="..\dfg\DFGNaiveDominators.h" />
     <ClInclude Include="..\dfg\DFGNaturalLoops.h" />

trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

@@ -417 +417 @@
                 0F885E111849A3BE00F1E3FA /* BytecodeUseDef.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F885E101849A3BE00F1E3FA /* BytecodeUseDef.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 0F893BDB1936E23C001211F4 /* DFGStructureAbstractValue.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F893BDA1936E23C001211F4 /* DFGStructureAbstractValue.cpp */; };
+                0F8F14331ADF090100ED792C /* DFGEpoch.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F8F142F1ADF090100ED792C /* DFGEpoch.cpp */; };
+                0F8F14341ADF090100ED792C /* DFGEpoch.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F8F14301ADF090100ED792C /* DFGEpoch.h */; settings = {ATTRIBUTES = (Private, ); }; };
+                0F8F14351ADF090100ED792C /* DFGMovHintRemovalPhase.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F8F14311ADF090100ED792C /* DFGMovHintRemovalPhase.cpp */; };
+                0F8F14361ADF090100ED792C /* DFGMovHintRemovalPhase.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F8F14321ADF090100ED792C /* DFGMovHintRemovalPhase.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 0F8F2B95172E04A0007DBDA5 /* FTLLink.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F8F2B93172E049E007DBDA5 /* FTLLink.cpp */; };
                 0F8F2B96172E04A3007DBDA5 /* FTLLink.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F8F2B94172E049E007DBDA5 /* FTLLink.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -2143 +2147 @@
                 0F885E101849A3BE00F1E3FA /* BytecodeUseDef.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BytecodeUseDef.h; sourceTree = "<group>"; };
                 0F893BDA1936E23C001211F4 /* DFGStructureAbstractValue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = DFGStructureAbstractValue.cpp; path = dfg/DFGStructureAbstractValue.cpp; sourceTree = "<group>"; };
+                0F8F142F1ADF090100ED792C /* DFGEpoch.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = DFGEpoch.cpp; path = dfg/DFGEpoch.cpp; sourceTree = "<group>"; };
+                0F8F14301ADF090100ED792C /* DFGEpoch.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGEpoch.h; path = dfg/DFGEpoch.h; sourceTree = "<group>"; };
+                0F8F14311ADF090100ED792C /* DFGMovHintRemovalPhase.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = DFGMovHintRemovalPhase.cpp; path = dfg/DFGMovHintRemovalPhase.cpp; sourceTree = "<group>"; };
+                0F8F14321ADF090100ED792C /* DFGMovHintRemovalPhase.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGMovHintRemovalPhase.h; path = dfg/DFGMovHintRemovalPhase.h; sourceTree = "<group>"; };
                 0F8F2B93172E049E007DBDA5 /* FTLLink.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = FTLLink.cpp; path = ftl/FTLLink.cpp; sourceTree = "<group>"; };
                 0F8F2B94172E049E007DBDA5 /* FTLLink.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = FTLLink.h; path = ftl/FTLLink.h; sourceTree = "<group>"; };
@@ -4907 +4915 @@
                                 A7D9A29117A0BC7400EE2618 /* DFGEdgeDominates.h */,
                                 A7986D5617A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h */,
+                                0F8F142F1ADF090100ED792C /* DFGEpoch.cpp */,
+                                0F8F14301ADF090100ED792C /* DFGEpoch.h */,
                                 A78A976C179738B8009DF744 /* DFGFailedFinalizer.cpp */,
                                 A78A976D179738B8009DF744 /* DFGFailedFinalizer.h */,
@@ -4966 +4976 @@
                                 0F2BDC4C1522818300CD8910 /* DFGMinifiedNode.cpp */,
                                 0F2BDC3E1522801700CD8910 /* DFGMinifiedNode.h */,
+                                0F8F14311ADF090100ED792C /* DFGMovHintRemovalPhase.cpp */,
+                                0F8F14321ADF090100ED792C /* DFGMovHintRemovalPhase.h */,
                                 0FC3CCFA19ADA410006AC72A /* DFGNaiveDominators.cpp */,
                                 0FC3CCFB19ADA410006AC72A /* DFGNaiveDominators.h */,
@@ -5661 +5673 @@
                                 C2FCAE1317A9C24E0034C735 /* BytecodeLivenessAnalysis.h in Headers */,
                                 0F666EC0183566F900D017F1 /* BytecodeLivenessAnalysisInlines.h in Headers */,
+                                0F8F14361ADF090100ED792C /* DFGMovHintRemovalPhase.h in Headers */,
                                 0F885E111849A3BE00F1E3FA /* BytecodeUseDef.h in Headers */,
                                 0F8023EA1613832B00A0BA45 /* ByValInfo.h in Headers */,
@@ -6294 +6307 @@
                                 0F34B14C16D43E0D001CDA5A /* PolymorphicAccessStructureList.h in Headers */,
                                 0F9FC8C414E1B60000D52AE0 /* PolymorphicPutByIdList.h in Headers */,
+                                0F8F14341ADF090100ED792C /* DFGEpoch.h in Headers */,
                                 0F98206116BFE38300240D02 /* PreciseJumpTargets.h in Headers */,
                                 0F2DD8151AB3D8BE00BBB8E8 /* DFGForAllKills.h in Headers */,
@@ -7376 +7390 @@
                                 86E3C61A167BABEE006D760A /* JSValue.mm in Sources */,
                                 0FF8BDEA1AD4CF7100DFE884 /* InferredValue.cpp in Sources */,
+                                0F8F14351ADF090100ED792C /* DFGMovHintRemovalPhase.cpp in Sources */,
                                 14BD5A320A3E91F600BAF59C /* JSValueRef.cpp in Sources */,
                                 147F39D7107EC37600427A48 /* JSEnvironmentRecord.cpp in Sources */,
@@ -7516 +7531 @@
                                 14469DEB107EC7E700650446 /* StringConstructor.cpp in Sources */,
                                 14469DEC107EC7E700650446 /* StringObject.cpp in Sources */,
+                                0F8F14331ADF090100ED792C /* DFGEpoch.cpp in Sources */,
                                 14469DED107EC7E700650446 /* StringPrototype.cpp in Sources */,
                                 0F2D4DEF19832DD3007D4B19 /* TypeSet.cpp in Sources */,

trunk/Source/JavaScriptCore/bytecode/CodeOrigin.cpp

@@ -197 +197 @@
         out.print(", known callee: ", inContext(calleeRecovery.constant(), context));
     out.print(", numArgs+this = ", arguments.size());
-    out.print(", stack < loc", VirtualRegister(stackOffset).toLocal());
-    out.print(">");
+    out.print(", stackOffset = ", stackOffset);
+    out.print(" (", virtualRegisterForLocal(0), " maps to ", virtualRegisterForLocal(0) + stackOffset, ")>");
 }
 

trunk/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp

@@ -144 +144 @@
                 
             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) {

trunk/Source/JavaScriptCore/dfg/DFGDisassembler.cpp

@@ -113 +113 @@
         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));

trunk/Source/JavaScriptCore/dfg/DFGMayExit.cpp

@@ -60 +60 @@
 {
     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:

trunk/Source/JavaScriptCore/dfg/DFGNodeType.h

@@ -64 +64 @@
     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. */ \

trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp

@@ -48 +48 @@
 #include "DFGLivenessAnalysisPhase.h"
 #include "DFGLoopPreHeaderCreationPhase.h"
+#include "DFGMovHintRemovalPhase.h"
 #include "DFGOSRAvailabilityAnalysisPhase.h"
 #include "DFGOSREntrypointCreationPhase.h"
@@ -393 +394 @@
         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);

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -1430 +1430 @@
         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(

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

@@ -1922 +1922 @@
     }
         
-    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;
     }

trunk/Source/JavaScriptCore/runtime/Options.h

@@ -185 +185 @@
     v(double, minimumCallToKnownRate, 0.51, nullptr) \
     v(bool, optimizeNativeCalls, false, nullptr) \
+    v(bool, enableMovHintRemoval, true, nullptr) \
     v(bool, enableObjectAllocationSinking, true, nullptr) \
     \