Attachment 206289 Details for Bug 118486 – the patch

[patch] the patch

blah.patch (text/plain), 4.38 KB, created by Filip Pizlo on 2013-07-08 22:03:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2013-07-08 22:03:22 PDT

Size: 4.38 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 152480)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,28 @@
>+2013-07-08  Filip Pizlo  <fpizlo@apple.com>
>+
>+        NaturalLoops + Profiler = Crash
>+        https://bugs.webkit.org/show_bug.cgi?id=118486
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        I borked dominators in:
>+        http://trac.webkit.org/changeset/152431/branches/dfgFourthTier/Source/JavaScriptCore/dfg/DFGDominators.h
>+        
>+        This patch also adds some debug support, and fixes the loop that adds a block to
>+        an already-existing natural loop. Note that we currently don't take that path in
>+        most programs, but it will arise, for example if you use 'continue' - though you'd
>+        have to use it rather cleverly since the bytecode will not jump to the loop header
>+        in most uses of 'continue'.
>+
>+        * dfg/DFGDominators.cpp:
>+        (JSC::DFG::Dominators::dump):
>+        (DFG):
>+        * dfg/DFGDominators.h:
>+        (JSC::DFG::Dominators::dominates):
>+        (Dominators):
>+        * dfg/DFGNaturalLoops.cpp:
>+        (JSC::DFG::NaturalLoops::compute):
>+
> 2013-07-08  Filip Pizlo  <fpizlo@apple.com>
> 
>         fourthTier: DFG::AbstractState::beginBasicBlock() should set m_haveStructures if any of the valuesAtHead have either a current known structure or a non-top/non-bottom array modes
>Index: Source/JavaScriptCore/dfg/DFGDominators.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGDominators.cpp	(revision 152480)
>+++ Source/JavaScriptCore/dfg/DFGDominators.cpp	(working copy)
>@@ -100,6 +100,22 @@ bool Dominators::iterateForBlock(Graph& 
>     return m_results[i].setAndCheck(m_scratch);
> }
> 
>+void Dominators::dump(Graph& graph, PrintStream& out) const
>+{
>+    for (BlockIndex blockIndex = 0; blockIndex < graph.numBlocks(); ++blockIndex) {
>+        BasicBlock* block = graph.block(blockIndex);
>+        if (!block)
>+            continue;
>+        out.print("    Block ", *block, ":");
>+        for (BlockIndex otherIndex = 0; otherIndex < graph.numBlocks(); ++otherIndex) {
>+            if (!dominates(block->index, otherIndex))
>+                continue;
>+            out.print(" #", otherIndex);
>+        }
>+        out.print("\n");
>+    }
>+}
>+
> } } // namespace JSC::DFG
> 
> #endif // ENABLE(DFG_JIT)
>Index: Source/JavaScriptCore/dfg/DFGDominators.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGDominators.h	(revision 152480)
>+++ Source/JavaScriptCore/dfg/DFGDominators.h	(working copy)
>@@ -49,7 +49,7 @@ public:
>     bool dominates(BlockIndex from, BlockIndex to) const
>     {
>         ASSERT(isValid());
>-        return m_results[from].get(to);
>+        return m_results[to].get(from);
>     }
>     
>     bool dominates(BasicBlock* from, BasicBlock* to) const
>@@ -57,10 +57,12 @@ public:
>         return dominates(from->index, to->index);
>     }
>     
>+    void dump(Graph& graph, PrintStream&) const;
>+    
> private:
>     bool iterateForBlock(Graph& graph, BlockIndex);
>     
>-    Vector<FastBitVector> m_results;
>+    Vector<FastBitVector> m_results; // For each block, the bitvector of blocks that dominate it.
>     FastBitVector m_scratch;
> };
> 
>Index: Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp	(revision 152480)
>+++ Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp	(working copy)
>@@ -58,6 +58,11 @@ void NaturalLoops::compute(Graph& graph)
>     
>     graph.m_dominators.computeIfNecessary(graph);
>     
>+    if (verbose) {
>+        dataLog("Dominators:\n");
>+        graph.m_dominators.dump(graph, WTF::dataFile());
>+    }
>+    
>     m_loops.resize(0);
>     
>     for (BlockIndex blockIndex = graph.numBlocks(); blockIndex--;) {
>@@ -71,9 +76,10 @@ void NaturalLoops::compute(Graph& graph)
>                 continue;
>             bool found = false;
>             for (unsigned j = m_loops.size(); j--;) {
>-                if (m_loops[i].header() == successor) {
>-                    m_loops[i].addBlock(block);
>+                if (m_loops[j].header() == successor) {
>+                    m_loops[j].addBlock(block);
>                     found = true;
>+                    break;
>                 }
>             }
>             if (found)

Flags:

ggaren: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 118486: 206289