Attachment 339102 Details for Bug 185126 – the patch

[patch] the patch

blah.patch (text/plain), 3.10 KB, created by Filip Pizlo on 2018-04-29 18:14:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2018-04-29 18:14:58 PDT

Size: 3.10 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 231154)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2018-04-29  Filip Pizlo  <fpizlo@apple.com>
>+
>+        LICM shouldn't hoist nodes if hoisted nodes exited in that code block
>+        https://bugs.webkit.org/show_bug.cgi?id=185126
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        I found this bug by accident when I was writing this test for something else.
>+        
>+        This change also speeds up other benchmarks of this case that we already had. They are all called
>+        the licm-dragons tests.
>+
>+        * microbenchmarks/licm-dragons-two-structures.js: Added.
>+        (foo):
>+
> 2018-04-29  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r231137.
>Index: JSTests/microbenchmarks/licm-dragons-two-structures.js
>===================================================================
>--- JSTests/microbenchmarks/licm-dragons-two-structures.js	(nonexistent)
>+++ JSTests/microbenchmarks/licm-dragons-two-structures.js	(working copy)
>@@ -0,0 +1,24 @@
>+function foo(o)
>+{
>+    var result = 0;
>+    for (var i = 0; i < 100; ++i) {
>+        if (o.p)
>+            result += o.f;
>+        else
>+            result += o.g;
>+        if (o.p)
>+            o.f = i;
>+        else
>+            o.g = i;
>+    }
>+    return result;
>+}
>+
>+noInline(foo);
>+
>+for (var i = 0; i < 100000; ++i) {
>+    var result = foo(i & 1 ? {p:true, f:42} : {p:false, g:42});
>+    if (result != (99 * 98) / 2 + 42)
>+        throw "Error: bad result: " + result;
>+}
>+
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 231154)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2018-04-29  Filip Pizlo  <fpizlo@apple.com>
>+
>+        LICM shouldn't hoist nodes if hoisted nodes exited in that code block
>+        https://bugs.webkit.org/show_bug.cgi?id=185126
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        This change is just restoring functionality that we've already had for a while. It had been
>+        accidentally broken due to an unrelated CodeBlock refactoring.
>+
>+        * dfg/DFGLICMPhase.cpp:
>+        (JSC::DFG::LICMPhase::attemptHoist):
>+
> 2018-04-29  Filip Pizlo  <fpizlo@apple.com>
> 
>         B3 should run tail duplication at the bitter end
>Index: Source/JavaScriptCore/dfg/DFGLICMPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGLICMPhase.cpp	(revision 231154)
>+++ Source/JavaScriptCore/dfg/DFGLICMPhase.cpp	(working copy)
>@@ -281,7 +281,7 @@ private:
>             && !m_graph.m_controlEquivalenceAnalysis->dominatesEquivalently(data.preHeader, fromBlock);
>         
>         if (addsBlindSpeculation
>-            && m_graph.hasExitSite(originalOrigin.semantic, HoistingFailed)) {
>+            && m_graph.hasGlobalExitSite(originalOrigin.semantic, HoistingFailed)) {
>             if (verbose) {
>                 dataLog(
>                     "    Not hoisting ", node, " because it may exit and the pre-header (",

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185126: 339102