Title: [266101] trunk/Source/_javascript_Core
Revision
266101
Author
keith_mil...@apple.com
Date
2020-08-24 21:32:13 -0700 (Mon, 24 Aug 2020)

Log Message

DFG should always run CFG Simplification after Constant Folding.
https://bugs.webkit.org/show_bug.cgi?id=215286

Reviewed by Robin Morisset.

We didn't do this originally because LICM, many years ago, was
unsound if the CFG didn't have exactly the right shape around
loops. This is no longer true so we don't have to worry about
changing the CFG anymore. While, this doesn't appear to be a
speedup on JetStream 2 CFG, probably because we'd eventually
simplify the graph in B3, CFG Simplification is very cheap and
make other DFG optimizations easier in the future.

Also, remove unecessary validation rule that no exitOKs can come
before any Phi nodes in DFG. This isn't required and fails after
merging two basic blocks where the latter block has a Phi.

* dfg/DFGCFGSimplificationPhase.cpp:
(JSC::DFG::CFGSimplificationPhase::run):
* dfg/DFGPlan.cpp:
(JSC::DFG::Plan::compileInThreadImpl):
* dfg/DFGValidate.cpp:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (266100 => 266101)


--- trunk/Source/_javascript_Core/ChangeLog	2020-08-25 03:36:45 UTC (rev 266100)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-08-25 04:32:13 UTC (rev 266101)
@@ -1,5 +1,30 @@
 2020-08-24  Keith Miller  <keith_mil...@apple.com>
 
+        DFG should always run CFG Simplification after Constant Folding.
+        https://bugs.webkit.org/show_bug.cgi?id=215286
+
+        Reviewed by Robin Morisset.
+
+        We didn't do this originally because LICM, many years ago, was
+        unsound if the CFG didn't have exactly the right shape around
+        loops. This is no longer true so we don't have to worry about
+        changing the CFG anymore. While, this doesn't appear to be a
+        speedup on JetStream 2 CFG, probably because we'd eventually
+        simplify the graph in B3, CFG Simplification is very cheap and
+        make other DFG optimizations easier in the future.
+
+        Also, remove unecessary validation rule that no exitOKs can come
+        before any Phi nodes in DFG. This isn't required and fails after
+        merging two basic blocks where the latter block has a Phi.
+
+        * dfg/DFGCFGSimplificationPhase.cpp:
+        (JSC::DFG::CFGSimplificationPhase::run):
+        * dfg/DFGPlan.cpp:
+        (JSC::DFG::Plan::compileInThreadImpl):
+        * dfg/DFGValidate.cpp:
+
+2020-08-24  Keith Miller  <keith_mil...@apple.com>
+
         Remove MovHintRemoval phase
         https://bugs.webkit.org/show_bug.cgi?id=215785
 

Modified: trunk/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp (266100 => 266101)


--- trunk/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp	2020-08-25 03:36:45 UTC (rev 266100)
+++ trunk/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp	2020-08-25 04:32:13 UTC (rev 266101)
@@ -49,9 +49,6 @@
     
     bool run()
     {
-        // FIXME: We should make this work in SSA. https://bugs.webkit.org/show_bug.cgi?id=148260
-        DFG_ASSERT(m_graph, nullptr, m_graph.m_form != SSA);
-        
         const bool extremeLogging = false;
 
         bool outerChanged = false;

Modified: trunk/Source/_javascript_Core/dfg/DFGPlan.cpp (266100 => 266101)


--- trunk/Source/_javascript_Core/dfg/DFGPlan.cpp	2020-08-25 03:36:45 UTC (rev 266100)
+++ trunk/Source/_javascript_Core/dfg/DFGPlan.cpp	2020-08-25 04:32:13 UTC (rev 266101)
@@ -364,6 +364,7 @@
     if (changed) {
         RUN_PHASE(performCFA);
         RUN_PHASE(performConstantFolding);
+        RUN_PHASE(performCFGSimplification);
     }
     
     // If we're doing validation, then run some analyses, to give them an opportunity
@@ -429,6 +430,7 @@
         RUN_PHASE(performLivenessAnalysis);
         RUN_PHASE(performCFA);
         RUN_PHASE(performConstantFolding);
+        RUN_PHASE(performCFGSimplification);
         RUN_PHASE(performCleanUp); // Reduce the graph size a lot.
         changed = false;
         RUN_PHASE(performStrengthReduction);
@@ -444,6 +446,7 @@
             RUN_PHASE(performLivenessAnalysis);
             RUN_PHASE(performCFA);
             RUN_PHASE(performConstantFolding);
+            RUN_PHASE(performCFGSimplification);
         }
         
         // Currently, this relies on pre-headers still being valid. That precludes running CFG

Modified: trunk/Source/_javascript_Core/dfg/DFGValidate.cpp (266100 => 266101)


--- trunk/Source/_javascript_Core/dfg/DFGValidate.cpp	2020-08-25 03:36:45 UTC (rev 266100)
+++ trunk/Source/_javascript_Core/dfg/DFGValidate.cpp	2020-08-25 04:32:13 UTC (rev 266101)
@@ -830,22 +830,16 @@
         for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
             VALIDATE((block), block->phis.isEmpty());
 
-            bool didSeeExitOK = false;
             bool isOSRExited = false;
             
             HashSet<Node*> nodesInThisBlock;
 
             for (auto* node : *block) {
-                didSeeExitOK |= node->origin.exitOK;
                 switch (node->op()) {
                 case Phi:
                     // Phi cannot exit, and it would be wrong to hoist anything to the Phi that could
                     // exit.
                     VALIDATE((node), !node->origin.exitOK);
-
-                    // It never makes sense to have exitOK anywhere in the block before a Phi. It's only
-                    // OK to exit after all Phis are done.
-                    VALIDATE((node), !didSeeExitOK);
                     break;
                     
                 case GetLocal:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to