LICM shouldn't hoist nodes if hoisted nodes exited in that code block

https://bugs.webkit.org/show_bug.cgi?id=185126

Reviewed by Saam Barati.

JSTests:

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):

Source/JavaScriptCore:

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231185 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 90a0f3d..b0c93f3 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -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 Saam Barati.
+        
+        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.

diff --git a/JSTests/microbenchmarks/licm-dragons-two-structures.js b/JSTests/microbenchmarks/licm-dragons-two-structures.js
new file mode 100644 (file)
index 0000000..0039bf2
--- /dev/null
+++ b/JSTests/microbenchmarks/licm-dragons-two-structures.js
@@ -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;
+}
+

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 4ea0399..be14673 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -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 Saam Barati.
+        
+        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-30  Mark Lam  <mark.lam@apple.com>
 
         Apply PtrTags to the MetaAllocator and friends.

diff --git a/Source/JavaScriptCore/dfg/DFGLICMPhase.cpp b/Source/JavaScriptCore/dfg/DFGLICMPhase.cpp
index f1f6489..8a9ebbb 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGLICMPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGLICMPhase.cpp
@@ -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 (",