Changeset 96482 in webkit

Timestamp:

Oct 2, 2011 7:13:11 PM (13 years ago)

Author:

fpizlo@apple.com

Message:

DFG misses some obvious opportunities for common subexpression elimination
​https://bugs.webkit.org/show_bug.cgi?id=69233

Reviewed by Oliver Hunt.

0.7% speed-up on SunSpider.

• dfg/DFGPropagator.cpp:

(JSC::DFG::Propagator::getByValLoadElimination):
(JSC::DFG::Propagator::getMethodLoadElimination):
(JSC::DFG::Propagator::checkStructureLoadElimination):
(JSC::DFG::Propagator::getByOffsetLoadElimination):
(JSC::DFG::Propagator::getPropertyStorageLoadElimination):
(JSC::DFG::Propagator::performNodeCSE):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGPropagator.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-10-02  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG misses some obvious opportunities for common subexpression elimination
+        https://bugs.webkit.org/show_bug.cgi?id=69233
+
+        Reviewed by Oliver Hunt.
+        
+        0.7% speed-up on SunSpider.
+
+        * dfg/DFGPropagator.cpp:
+        (JSC::DFG::Propagator::getByValLoadElimination):
+        (JSC::DFG::Propagator::getMethodLoadElimination):
+        (JSC::DFG::Propagator::checkStructureLoadElimination):
+        (JSC::DFG::Propagator::getByOffsetLoadElimination):
+        (JSC::DFG::Propagator::getPropertyStorageLoadElimination):
+        (JSC::DFG::Propagator::performNodeCSE):
+
 2011-10-02  Gavin Barraclough  <barraclough@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGPropagator.cpp

@@ -965 +965 @@
                     return node.child3();
                 break;
+            case PutStructure:
+            case PutByOffset:
+                // GetByVal currently always speculates that it's accessing an
+                // array with an integer index, which means that it's impossible
+                // for a structure change or a put to property storage to affect
+                // the GetByVal.
+                break;
             default:
-                break;
-            }
-            if (clobbersWorld(index))
-                break;
+                if (clobbersWorld(index))
+                    return NoNode;
+                break;
+            }
         }
         return NoNode;
@@ -979 +986 @@
         for (NodeIndex index = m_compileIndex; index-- > start;) {
             Node& node = m_graph[index];
-            if (node.op == CheckMethod
-                && node.child1() == child1
-                && node.identifierNumber() == identifierNumber
-                && m_graph.m_methodCheckData[node.methodCheckDataIndex()] == methodCheckData)
-                return index;
-            if (clobbersWorld(index))
-                break;
+            switch (node.op) {
+            case CheckMethod:
+                if (node.child1() == child1
+                    && node.identifierNumber() == identifierNumber
+                    && m_graph.m_methodCheckData[node.methodCheckDataIndex()] == methodCheckData)
+                    return index;
+                break;
+                
+            case PutByOffset:
+                // If a put was optimized to by-offset then it's not changing the structure
+                break;
+                
+            case PutByVal:
+            case PutByValAlias:
+                // PutByVal currently always speculates that it's accessing an array with an
+                // integer index, which means that it's impossible for it to cause a structure
+                // change.
+                break;
+                
+            default:
+                if (clobbersWorld(index))
+                    return NoNode;
+                break;
+            }
         }
         return NoNode;
@@ -1012 +1036 @@
                 break;
                 
+            case PutByVal:
+            case PutByValAlias:
+                // PutByVal currently always speculates that it's accessing an array with an
+                // integer index, which means that it's impossible for it to cause a structure
+                // change.
+                break;
+                
             default:
                 if (clobbersWorld(index))
@@ -1045 +1076 @@
                 break;
                 
+            case PutByVal:
+            case PutByValAlias:
+                // PutByVal currently always speculates that it's accessing an array with an
+                // integer index, which means that it's impossible for it to cause a structure
+                // change.
+                break;
+                
             default:
                 if (clobbersWorld(index))
@@ -1069 +1107 @@
                 // Changing the structure or putting to the storage cannot
                 // change the property storage pointer.
+                break;
+                
+            case PutByVal:
+            case PutByValAlias:
+                // PutByVal currently always speculates that it's accessing an array with an
+                // integer index, which means that it's impossible for it to cause a structure
+                // change.
                 break;
                 
@@ -1192 +1237 @@
         case ArithSqrt:
         case GetCallee:
-        case GetArrayLength:
         case GetStringLength:
             setReplacement(pureCSE(node));
+            break;
+            
+        case GetArrayLength:
+            setReplacement(impureCSE(node));
             break;