Attachment 349979 Details for Bug 189682 – patch

[patch] patch

a-backup.diff (text/plain), 4.46 KB, created by Saam Barati on 2018-09-17 17:29:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-09-17 17:29:20 PDT

Size: 4.46 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 236094)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,25 @@
>+2018-09-17  Saam barati  <sbarati@apple.com>
>+
>+        DFGValidate should only validate that edges have a result in SSA if we haven't proven that we have OSR exited
>+        https://bugs.webkit.org/show_bug.cgi?id=189682
>+        <rdar://problem/43557315>
>+
>+        Reviewed by Mark Lam.
>+
>+        * stress/arguments-elimination-will-generate-edge-without-result.js: Added.
>+        (foo):
>+
>+2018-09-17  Saam barati  <sbarati@apple.com>
>+
>+        ArgumentsEliminationPhase should snip basic blocks after proven OSR exits
>+        https://bugs.webkit.org/show_bug.cgi?id=189682
>+        <rdar://problem/43557315>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/arguments-elimination-will-generate-edge-without-result.js: Added.
>+        (foo):
>+
> 2018-09-17  Saam barati  <sbarati@apple.com>
> 
>         DFGValidate should only validate that edges have a result in SSA if we haven't proven that we have OSR exited
>Index: JSTests/stress/arguments-elimination-will-generate-edge-without-result.js
>===================================================================
>--- JSTests/stress/arguments-elimination-will-generate-edge-without-result.js	(nonexistent)
>+++ JSTests/stress/arguments-elimination-will-generate-edge-without-result.js	(working copy)
>@@ -0,0 +1,9 @@
>+//@ runDefault("--validateGraphAtEachPhase=true", "--jitPolicyScale=0", "--useConcurrentJIT=0")
>+
>+'use strict';
>+function foo() {
>+  return arguments[1][0] === arguments[0];
>+}
>+for (let i = 0; i < 100000; ++i) {
>+  foo(0, 0);
>+}
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 236078)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,39 @@
>+2018-09-17  Saam barati  <sbarati@apple.com>
>+
>+        ArgumentsEliminationPhase should snip basic blocks after proven OSR exits
>+        https://bugs.webkit.org/show_bug.cgi?id=189682
>+        <rdar://problem/43557315>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Otherwise, if we have code like this:
>+        ```
>+        a: Arguments
>+        b: GetButterfly(@a)
>+        c: ForceExit
>+        d: GetArrayLength(@a, @b)
>+        ```
>+        it will get transformed into this invalid DFG IR:
>+        ```
>+        a: PhantomArguments
>+        b: Check(@a)
>+        c: ForceExit
>+        d: GetArrayLength(@a, @b)
>+        ```
>+        
>+        And we will fail DFG validation since @b does not have a result.
>+        
>+        The fix is to just remove all nodes after the ForceExit and plant an
>+        Unreachable after it. So the above code program will now turn into this:
>+        ```
>+        a: PhantomArguments
>+        b: Check(@a)
>+        c: ForceExit
>+        e: Unreachable
>+        ```
>+
>+        * dfg/DFGArgumentsEliminationPhase.cpp:
>+
> 2018-09-17  Darin Adler  <darin@apple.com>
> 
>         Use OpaqueJSString rather than JSRetainPtr inside WebKit
>Index: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp	(revision 236078)
>+++ Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp	(working copy)
>@@ -627,6 +627,7 @@ private:
>         InsertionSet insertionSet(m_graph);
>         
>         for (BasicBlock* block : m_graph.blocksInPreOrder()) {
>+            Node* pseudoTerminal = nullptr;
>             for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
>                 Node* node = block->at(nodeIndex);
>                 
>@@ -1210,11 +1211,25 @@ private:
>                 default:
>                     break;
>                 }
>-                if (node->isPseudoTerminal())
>+
>+                if (node->isPseudoTerminal()) {
>+                    pseudoTerminal = node;
>                     break;
>+                }
>             }
>-            
>+
>             insertionSet.execute(block);
>+
>+            if (pseudoTerminal) {
>+                for (unsigned i = 0; i < block->size(); ++i) {
>+                    Node* node = block->at(i);
>+                    if (node != pseudoTerminal)
>+                        continue;
>+                    block->resize(i + 1);
>+                    block->append(m_graph.addNode(SpecNone, Unreachable, node->origin));
>+                    break;
>+                }
>+            }
>         }
>     }
>

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 189682: 349947 | 349950 | 349979 | 350298