Unreviewed, fix some FIXMEs and add some new ones, based on things we've learned...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Aug 2015 23:41:59 +0000 (23:41 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Aug 2015 23:41:59 +0000 (23:41 +0000)
recent OSR exit work.

* dfg/DFGLICMPhase.cpp:
(JSC::DFG::LICMPhase::run):
(JSC::DFG::LICMPhase::attemptHoist):
* dfg/DFGMayExit.cpp:
* dfg/DFGMayExit.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@189070 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGLICMPhase.cpp
Source/JavaScriptCore/dfg/DFGMayExit.cpp
Source/JavaScriptCore/dfg/DFGMayExit.h

index 5783893..bd4f469 100644 (file)
@@ -1,3 +1,14 @@
+2015-08-27  Filip Pizlo  <fpizlo@apple.com>
+
+        Unreviewed, fix some FIXMEs and add some new ones, based on things we've learned from some
+        recent OSR exit work.
+
+        * dfg/DFGLICMPhase.cpp:
+        (JSC::DFG::LICMPhase::run):
+        (JSC::DFG::LICMPhase::attemptHoist):
+        * dfg/DFGMayExit.cpp:
+        * dfg/DFGMayExit.h:
+
 2015-08-27  Keith Miller  <keith_miller@apple.com>
 
         [ES6] Add TypedArray.prototype functionality.
index 66f5742..57937e2 100644 (file)
@@ -130,11 +130,12 @@ public:
             
             DFG_ASSERT(m_graph, preHeader->terminal(), preHeader->terminal()->op() == Jump);
             
-            // We should validate the pre-header. If we placed forExit origins on nodes only if
-            // at the top of that node it is legal to exit, then we would simply check if Jump
-            // had a forExit. We should disable hoisting to pre-headers that don't validate.
-            // Or, we could only allow hoisting of things that definitely don't exit.
-            // FIXME: https://bugs.webkit.org/show_bug.cgi?id=145204
+            // We should validate the pre-header. This currently assumes that it's valid to OSR exit at
+            // the Jump of the pre-header. That may not always be the case, like if we lowered a Node
+            // that was ExitInvalid to a loop. This phase should somehow defend against this - at the
+            // very least with assertions, if not with something better (like only hoisting things that
+            // cannot exit).
+            // FIXME: https://bugs.webkit.org/show_bug.cgi?id=148259
             
             data.preHeader = preHeader;
         }
@@ -283,16 +284,18 @@ private:
                 "    Hoisting ", node, " from ", *fromBlock, " to ", *data.preHeader,
                 "\n");
         }
-        
+
+        // FIXME: We should adjust the Check: flags on the edges of node. There are phases that assume
+        // that those flags are correct even if AI is stale.
+        // https://bugs.webkit.org/show_bug.cgi?id=148544
         data.preHeader->insertBeforeTerminal(node);
         node->owner = data.preHeader;
         NodeOrigin originalOrigin = node->origin;
         node->origin = data.preHeader->terminal()->origin.withSemantic(node->origin.semantic);
         
         // Modify the states at the end of the preHeader of the loop we hoisted to,
-        // and all pre-headers inside the loop.
-        // FIXME: This could become a scalability bottleneck. Fortunately, most loops
-        // are small and anyway we rapidly skip over basic blocks here.
+        // and all pre-headers inside the loop. This isn't a stability bottleneck right now
+        // because most loops are small and most blocks belong to few loops.
         for (unsigned bodyIndex = loop->size(); bodyIndex--;) {
             BasicBlock* subBlock = loop->at(bodyIndex);
             const NaturalLoop* subLoop = m_graph.m_naturalLoops.headerOf(subBlock);
index 7c4ecd8..8259a97 100644 (file)
@@ -45,6 +45,8 @@ public:
     
     void operator()(Node*, Edge edge)
     {
+        // FIXME: Maybe this should call mayHaveTypeCheck(edge.useKind()) instead.
+        // https://bugs.webkit.org/show_bug.cgi?id=148545
         if (edge.willHaveCheck()) {
             m_result = true;
             return;
index 011bc56..7cece0d 100644 (file)
@@ -66,6 +66,12 @@ enum ExitMode {
     Exits
 };
 
+// FIXME: This currently consumes the Check: flag produced by AI, and will claim that something doesn't
+// exit if the Check: flag was cleared. This makes it hard to use mayExit() for things like hoisting
+// (for example in LICM), since that wants to know if the node would exit if it was moved somewhere
+// else.
+// https://bugs.webkit.org/show_bug.cgi?id=148545
+
 ExitMode mayExit(Graph&, Node*);
 
 } } // namespace JSC::DFG