Changeset 181495 in webkit

Timestamp:

Mar 13, 2015, 7:50:36 PM (11 years ago)

Author:

fpizlo@apple.com

Message:

Object allocation sinking phase shouldn't re-decorate previously sunken allocations on each fixpoint operation
​https://bugs.webkit.org/show_bug.cgi?id=142686

Reviewed by Oliver Hunt.

Just because promoteHeapAccess() notifies us of an effect to a heap location in a node doesn't
mean that we should handle it as if it was for one of our sinking candidates. Instead we should
prune based on m_sinkCandidates.

This fixes a benign bug where we would generate a lot of repeated IR for some pathological
tests.

• dfg/DFGObjectAllocationSinkingPhase.cpp:

(JSC::DFG::ObjectAllocationSinkingPhase::promoteSunkenFields):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGObjectAllocationSinkingPhase.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-03-13  Filip Pizlo  <fpizlo@apple.com>
+
+        Object allocation sinking phase shouldn't re-decorate previously sunken allocations on each fixpoint operation
+        https://bugs.webkit.org/show_bug.cgi?id=142686
+
+        Reviewed by Oliver Hunt.
+        
+        Just because promoteHeapAccess() notifies us of an effect to a heap location in a node doesn't
+        mean that we should handle it as if it was for one of our sinking candidates. Instead we should
+        prune based on m_sinkCandidates.
+        
+        This fixes a benign bug where we would generate a lot of repeated IR for some pathological
+        tests.
+
+        * dfg/DFGObjectAllocationSinkingPhase.cpp:
+        (JSC::DFG::ObjectAllocationSinkingPhase::promoteSunkenFields):
+
 2015-03-13  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp

@@ -582 +582 @@
                     node,
                     [&] (PromotedHeapLocation location, Edge) {
-                        locations.add(location);
+                        if (m_sinkCandidates.contains(location.base()))
+                            locations.add(location);
                     },
                     [&] (PromotedHeapLocation location) {
-                        locations.add(location);
+                        if (m_sinkCandidates.contains(location.base()))
+                            locations.add(location);
                     });
             }
@@ -637 +639 @@
                     node,
                     [&] (PromotedHeapLocation location, Edge value) {
+                        if (!m_sinkCandidates.contains(location.base()))
+                            return;
                         SSACalculator::Variable* variable = m_locationToVariable.get(location);
                         m_ssaCalculator.newDef(variable, block, value.node());
@@ -688 +692 @@
                     node,
                     [&] (PromotedHeapLocation location, Edge value) {
-                        m_localMapping.set(location, value.node());
+                        if (m_sinkCandidates.contains(location.base()))
+                            m_localMapping.set(location, value.node());
                     },
                     [&] (PromotedHeapLocation location) {
-                        node->replaceWith(resolve(block, location));
+                        if (m_sinkCandidates.contains(location.base()))
+                            node->replaceWith(resolve(block, location));
                     });
             }