DFG is still too pessimistic about what constitutes a side-effect on array accesses
https://bugs.webkit.org/show_bug.cgi?id=94309

Reviewed by Geoffrey Garen.

This change means that even if structure transition watchpoints are not used for
hoisting of clobbered structure checks, we still retain good performance on the
benchmarks we care about. That's important, since butterflies will likely make
most array structures not watchpointable.

* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::execute):
* dfg/DFGStructureCheckHoistingPhase.cpp:
(JSC::DFG::StructureCheckHoistingPhase::run):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@125959 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 436cc10..2522b03 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
+2012-08-17  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG is still too pessimistic about what constitutes a side-effect on array accesses
+        https://bugs.webkit.org/show_bug.cgi?id=94309
+
+        Reviewed by Geoffrey Garen.
+
+        This change means that even if structure transition watchpoints are not used for
+        hoisting of clobbered structure checks, we still retain good performance on the
+        benchmarks we care about. That's important, since butterflies will likely make
+        most array structures not watchpointable.
+
+        * dfg/DFGAbstractState.cpp:
+        (JSC::DFG::AbstractState::execute):
+        * dfg/DFGStructureCheckHoistingPhase.cpp:
+        (JSC::DFG::StructureCheckHoistingPhase::run):
+
 2012-08-17  Milian Wolff  <milian.wolff@kdab.com>
 
         [Qt] QNX build fails due to ctype usage in system headers
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
index e09b940..67bdb47 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
+++ b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
@@ -956,7 +956,7 @@
             m_isValid = false;
             break;
         }
-        if (!m_graph[child2].shouldSpeculateInteger() || !isActionableMutableArraySpeculation(m_graph[child1].prediction())
+        if (!m_graph[child2].shouldSpeculateInteger()
 #if USE(JSVALUE32_64)
             || m_graph[child1].shouldSpeculateArguments()
 #endif
diff --git a/Source/JavaScriptCore/dfg/DFGStructureCheckHoistingPhase.cpp b/Source/JavaScriptCore/dfg/DFGStructureCheckHoistingPhase.cpp
index 3d09f8d..9a99817 100644
--- a/Source/JavaScriptCore/dfg/DFGStructureCheckHoistingPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGStructureCheckHoistingPhase.cpp
@@ -97,6 +97,7 @@
                 case GetByVal:
                 case PutByVal:
                 case PutByValAlias:
+                case PutByValSafe:
                 case GetArrayLength:
                 case Phantom:
                     // Don't count these uses.
@@ -220,17 +221,22 @@
                     break;
                     
                 case PutByVal:
-                case PutByValAlias: {
+                case PutByValAlias:
+                case PutByValSafe: {
                     Edge child1 = m_graph.varArgChild(node, 0);
                     Edge child2 = m_graph.varArgChild(node, 1);
                     
                     if (!m_graph[child1].prediction() || !m_graph[child2].prediction())
                         break;
-                    if (!m_graph[child2].shouldSpeculateInteger() || !isActionableMutableArraySpeculation(m_graph[child1].prediction())) {
+                    if (!m_graph[child2].shouldSpeculateInteger()
+#if USE(JSVALUE32_64)
+                        || m_graph[child1].shouldSpeculateArguments()
+#endif
+                        ) {
                         clobber(live);
                         break;
                     }
-                    if (node.op() == PutByValAlias)
+                    if (node.op() != PutByValSafe)
                         break;
                     if (m_graph[child1].shouldSpeculateArguments())
                         break;
@@ -296,7 +302,7 @@
             dataLog("Hoisting checks for %s\n", m_graph.nameOfVariableAccessData(it->first));
         }
 #endif // DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
-
+        
         // Make changes:
         // 1) If a variable's live range does not span a clobber, then inject structure
         //    checks before the SetLocal.