[webkit-changes] [283862] trunk/Source/JavaScriptCore

Fri, 08 Oct 2021 20:30:41 -0700

Title: [283862] trunk/Source/_javascript_Core
Revision
283862
Author
[email protected]
Date
2021-10-08 20:29:53 -0700 (Fri, 08 Oct 2021)

Log Message

Run backwards propagation before we prune the graph after ForceOSRExit nodes in BytecodeParser
https://bugs.webkit.org/show_bug.cgi?id=230823
<rdar://problem/83565088>

Reviewed by Yusuke Suzuki.

When I ported the phase to run right after bytecode parsing, I wanted
to maintain the same behavior as the prior pass that ran after CPS
rethreading. I noticed a slight bug in some of my logic that changed
some of heuristics and how they'd effect double voting.

The old patch was mimicking the "is loaded from" bit by using the NodeFlags.
Howver, this has some issues with how this interacts with our other uses
of NodeFlags. So, to make things simple, I just add a new "VariableIsUsed"
bit.

* dfg/DFGBackwardsPropagationPhase.cpp:
(JSC::DFG::BackwardsPropagationPhase::propagate):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (283861 => 283862)


--- trunk/Source/_javascript_Core/ChangeLog	2021-10-09 02:53:27 UTC (rev 283861)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-10-09 03:29:53 UTC (rev 283862)
@@ -1,3 +1,24 @@
+2021-10-08  Saam Barati  <[email protected]>
+
+        Run backwards propagation before we prune the graph after ForceOSRExit nodes in BytecodeParser
+        https://bugs.webkit.org/show_bug.cgi?id=230823
+        <rdar://problem/83565088>
+
+        Reviewed by Yusuke Suzuki.
+
+        When I ported the phase to run right after bytecode parsing, I wanted
+        to maintain the same behavior as the prior pass that ran after CPS
+        rethreading. I noticed a slight bug in some of my logic that changed
+        some of heuristics and how they'd effect double voting.
+        
+        The old patch was mimicking the "is loaded from" bit by using the NodeFlags.
+        Howver, this has some issues with how this interacts with our other uses
+        of NodeFlags. So, to make things simple, I just add a new "VariableIsUsed"
+        bit.
+
+        * dfg/DFGBackwardsPropagationPhase.cpp:
+        (JSC::DFG::BackwardsPropagationPhase::propagate):
+
 2021-10-08  Tadeu Zagallo  <[email protected]> and Keith Miller  <[email protected]>
 
         Implement the WebAssembly exception handling proposal

Modified: trunk/Source/_javascript_Core/dfg/DFGBackwardsPropagationPhase.cpp (283861 => 283862)


--- trunk/Source/_javascript_Core/dfg/DFGBackwardsPropagationPhase.cpp	2021-10-09 02:53:27 UTC (rev 283861)
+++ trunk/Source/_javascript_Core/dfg/DFGBackwardsPropagationPhase.cpp	2021-10-09 03:29:53 UTC (rev 283862)
@@ -32,6 +32,7 @@
 #include "DFGGraph.h"
 #include "DFGPhase.h"
 #include "JSCJSValueInlines.h"
+#include <wtf/MathExtras.h>
 
 namespace JSC { namespace DFG {
 
@@ -45,7 +46,7 @@
         , m_flagsAtHead(graph)
     {
     }
-    
+
     bool run()
     {
         for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
@@ -214,6 +215,10 @@
         return changed;
     }
     
+    static constexpr NodeFlags VariableIsUsed = 1 << (1 + WTF::getMSBSetConstexpr(NodeBytecodeBackPropMask));
+    static_assert(!(VariableIsUsed & NodeBytecodeBackPropMask));
+    static_assert(VariableIsUsed > NodeBytecodeBackPropMask, "Verify the above doesn't overflow");
+    
     void propagate(Node* node)
     {
         NodeFlags flags = node->flags() & NodeBytecodeBackPropMask;
@@ -221,9 +226,9 @@
         switch (node->op()) {
         case GetLocal: {
             VariableAccessData* variableAccessData = node->variableAccessData();
-            NodeFlags& flagsRef = m_currentFlags.operand(variableAccessData->operand());
-            mergeFlags(flagsRef, flags);
-            variableAccessData->mergeFlags(flagsRef & ~NodeBytecodeUsesAsInt); // We don't care about cross-block uses-as-int for this.
+            flags |= m_currentFlags.operand(variableAccessData->operand());
+            flags |= VariableIsUsed;
+            m_currentFlags.operand(variableAccessData->operand()) = flags;
             break;
         }
             
@@ -232,10 +237,11 @@
 
             Operand operand = variableAccessData->operand();
             NodeFlags flags = m_currentFlags.operand(operand);
-            if (!flags)
+            if (!(flags & VariableIsUsed))
                 break;
 
-            RELEASE_ASSERT(!(flags & ~NodeBytecodeBackPropMask));
+            flags &= NodeBytecodeBackPropMask;
+            flags &= ~NodeBytecodeUsesAsInt; // We don't care about cross-block uses-as-int.
 
             variableAccessData->mergeFlags(flags);
             // We union with NodeBytecodeUsesAsNumber to account for the fact that control flow may cause overflows that our modeling can't handle.
@@ -248,11 +254,15 @@
             
         case Flush: {
             VariableAccessData* variableAccessData = node->variableAccessData();
-            NodeFlags& flagsRef = m_currentFlags.operand(variableAccessData->operand());
-            mergeFlags(flagsRef, NodeBytecodeUsesAsValue);
-            variableAccessData->mergeFlags(flagsRef);
+            mergeFlags(m_currentFlags.operand(variableAccessData->operand()), NodeBytecodeUsesAsValue | VariableIsUsed);
             break;
         }
+
+        case PhantomLocal: {
+            VariableAccessData* variableAccessData = node->variableAccessData();
+            mergeFlags(m_currentFlags.operand(variableAccessData->operand()), VariableIsUsed);
+            break;
+        }
             
         case MovHint:
         case Check:

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to