Attachment 215990 Details for Bug 123779 – the patch

[patch] the patch

blah.patch (text/plain), 5.39 KB, created by Filip Pizlo on 2013-11-04 20:24:32 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2013-11-04 20:24:32 PST

Size: 5.39 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 158639)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,24 @@
>+2013-11-04  Filip Pizlo  <fpizlo@apple.com>
>+
>+        DFG CheckArray(String) should just be a Phantom(String:)
>+        https://bugs.webkit.org/show_bug.cgi?id=123779
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        This should be a speed-up since Phantom(String:) is smart enough to use the string
>+        structure. It should also be a simplification since CheckArray(String) was totally
>+        redundant.
>+        
>+        Also FixupPhase was assuming that it may see CheckArray's. That's wrong. It can
>+        create CheckArray's but it won't see them as input since no previous phase can
>+        create them.
>+
>+        * dfg/DFGFixupPhase.cpp:
>+        (JSC::DFG::FixupPhase::fixupNode):
>+        (JSC::DFG::FixupPhase::checkArray):
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::checkArray):
>+
> 2013-11-04  Filip Pizlo  <fpizlo@apple.com>
> 
>         DFG NewArray/NewArrayBuffer shouldn't be constructing with negative indexing
>Index: Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGFixupPhase.cpp	(revision 158630)
>+++ Source/JavaScriptCore/dfg/DFGFixupPhase.cpp	(working copy)
>@@ -806,18 +806,6 @@ private:
>             break;
>         }
>             
>-        case CheckArray: {
>-            switch (node->arrayMode().type()) {
>-            case Array::String:
>-                fixEdge<StringUse>(node->child1());
>-                break;
>-            default:
>-                fixEdge<CellUse>(node->child1());
>-                break;
>-            }
>-            break;
>-        }
>-            
>         case Arrayify:
>         case ArrayifyToStructure: {
>             fixEdge<CellUse>(node->child1());
>@@ -888,6 +876,7 @@ private:
>         case Int52ToDouble:
>         case Int52ToValue:
>         case InvalidationPoint:
>+        case CheckArray:
>             RELEASE_ASSERT_NOT_REACHED();
>             break;
> 
>@@ -1275,29 +1264,35 @@ private:
>     {
>         ASSERT(arrayMode.isSpecific());
>         
>-        Structure* structure = arrayMode.originalArrayStructure(m_graph, codeOrigin);
>+        if (arrayMode.type() == Array::String) {
>+            m_insertionSet.insertNode(
>+                m_indexInBlock, SpecNone, Phantom, codeOrigin,
>+                Edge(array, StringUse));
>+        } else {
>+            Structure* structure = arrayMode.originalArrayStructure(m_graph, codeOrigin);
>         
>-        Edge indexEdge = index ? Edge(index, Int32Use) : Edge();
>+            Edge indexEdge = index ? Edge(index, Int32Use) : Edge();
>         
>-        if (arrayMode.doesConversion()) {
>-            if (structure) {
>-                m_insertionSet.insertNode(
>-                    m_indexInBlock, SpecNone, ArrayifyToStructure, codeOrigin,
>-                    OpInfo(structure), OpInfo(arrayMode.asWord()), Edge(array, CellUse), indexEdge);
>-            } else {
>-                m_insertionSet.insertNode(
>-                    m_indexInBlock, SpecNone, Arrayify, codeOrigin,
>-                    OpInfo(arrayMode.asWord()), Edge(array, CellUse), indexEdge);
>-            }
>-        } else {
>-            if (structure) {
>-                m_insertionSet.insertNode(
>-                    m_indexInBlock, SpecNone, CheckStructure, codeOrigin,
>-                    OpInfo(m_graph.addStructureSet(structure)), Edge(array, CellUse));
>+            if (arrayMode.doesConversion()) {
>+                if (structure) {
>+                    m_insertionSet.insertNode(
>+                        m_indexInBlock, SpecNone, ArrayifyToStructure, codeOrigin,
>+                        OpInfo(structure), OpInfo(arrayMode.asWord()), Edge(array, CellUse), indexEdge);
>+                } else {
>+                    m_insertionSet.insertNode(
>+                        m_indexInBlock, SpecNone, Arrayify, codeOrigin,
>+                        OpInfo(arrayMode.asWord()), Edge(array, CellUse), indexEdge);
>+                }
>             } else {
>-                m_insertionSet.insertNode(
>-                    m_indexInBlock, SpecNone, CheckArray, codeOrigin,
>-                    OpInfo(arrayMode.asWord()), Edge(array, CellUse));
>+                if (structure) {
>+                    m_insertionSet.insertNode(
>+                        m_indexInBlock, SpecNone, CheckStructure, codeOrigin,
>+                        OpInfo(m_graph.addStructureSet(structure)), Edge(array, CellUse));
>+                } else {
>+                    m_insertionSet.insertNode(
>+                        m_indexInBlock, SpecNone, CheckArray, codeOrigin,
>+                        OpInfo(arrayMode.asWord()), Edge(array, CellUse));
>+                }
>             }
>         }
>         
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(revision 158630)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(working copy)
>@@ -751,7 +751,7 @@ void SpeculativeJIT::checkArray(Node* no
>     
>     switch (node->arrayMode().type()) {
>     case Array::String:
>-        expectedClassInfo = JSString::info();
>+        RELEASE_ASSERT_NOT_REACHED(); // Should have been a Phantom(String:)
>         break;
>     case Array::Int32:
>     case Array::Double:

Flags:

ggaren: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 123779: 215990