Attachment 339404 Details for Bug 178389 – Patch

[patch] Patch

bug-178389-20180503153748.patch (text/plain), 5.88 KB, created by Robin Morisset on 2018-05-03 06:37:49 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2018-05-03 06:37:49 PDT

Size: 5.88 KB

patch

obsolete

>Subversion Revision: 231301
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 24706688fbd1e0be8f4ad81701ea561c4f83a7be..cc8e7770b2527de9ffb7e5d47f462431f8f43881 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-05-03  Robin Morisset  <rmorisset@apple.com>
>+
>+        Enable tier-up in loops created by recursive tail call optimizations.
>+        https://bugs.webkit.org/show_bug.cgi?id=178389
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Made the recursive tail call optimization only run in FTL mode, since it is a significant progression on TailBench.
>+        Also some trivial refactoring of handleRecursiveTailCall.
>+
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::handleRecursiveTailCall):
>+
> 2018-05-03  Dominik Infuehr  <dinfuehr@igalia.com>
> 
>         Disable usage of fused multiply-add instructions for JSC with compiler flag
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index 4f3ede1da1fb7f16f551f2348888da3893e6c5a4..074a3ff346ecbf893394be19e53cf1720e556d0c 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -1361,6 +1361,11 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c
>     if (UNLIKELY(!Options::optimizeRecursiveTailCalls()))
>         return false;
> 
>+    // This optimisation brings more performance if it only runs in FTL, probably because it interferes with tier-up.
>+    // See https://bugs.webkit.org/show_bug.cgi?id=178389 for details.
>+    if (!isFTL(m_graph.m_plan.mode))
>+        return false;
>+
>     auto targetExecutable = callVariant.executable();
>     InlineStackEntry* stackEntry = m_inlineStackTop;
>     do {
>@@ -1373,6 +1378,10 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c
>             // We "continue" instead of returning false in case another stack entry further on the stack has the right number of arguments.
>             if (argumentCountIncludingThis != static_cast<int>(callFrame->argumentCountIncludingThis))
>                 continue;
>+            // If an InlineCallFrame is not a closure, it was optimized using a constant callee.
>+            // Check if this is the same callee that we are dealing with.
>+            if (!callFrame->isClosureCall && callFrame->calleeConstant() != callVariant.function())
>+                continue;
>         } else {
>             // We are in the machine code entry (i.e. the original caller).
>             // If we have more arguments than the number of parameters to the function, it is not clear where we could put them on the stack.
>@@ -1380,24 +1389,16 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c
>                 return false;
>         }
> 
>-        // If an InlineCallFrame is not a closure, it was optimized using a constant callee.
>-        // Check if this is the same callee that we try to inline here.
>-        if (stackEntry->m_inlineCallFrame && !stackEntry->m_inlineCallFrame->isClosureCall) {
>-            if (stackEntry->m_inlineCallFrame->calleeConstant() != callVariant.function())
>-                continue;
>-        }
>-
>         // We must add some check that the profiling information was correct and the target of this call is what we thought.
>         emitFunctionCheckIfNeeded();
>         // We flush everything, as if we were in the backedge of a loop (see treatment of op_jmp in parseBlock).
>         flushForTerminal();
> 
>         // We must set the callee to the right value
>-        if (stackEntry->m_inlineCallFrame) {
>-            if (stackEntry->m_inlineCallFrame->isClosureCall)
>-                setDirect(stackEntry->remapOperand(VirtualRegister(CallFrameSlot::callee)), callTargetNode, NormalSet);
>-        } else
>+        if (!stackEntry->m_inlineCallFrame)
>             addToGraph(SetCallee, callTargetNode);
>+        else if (stackEntry->m_inlineCallFrame->isClosureCall)
>+            setDirect(stackEntry->remapOperand(VirtualRegister(CallFrameSlot::callee)), callTargetNode, NormalSet);
> 
>         // We must set the arguments to the right values
>         if (!stackEntry->m_inlineCallFrame)
>@@ -1434,7 +1435,6 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c
>         // It would be unsound to jump over a non-tail call: the "tail" call is not really a tail call in that case.
>     } while (stackEntry->m_inlineCallFrame && stackEntry->m_inlineCallFrame->kind == InlineCallFrame::TailCall && (stackEntry = stackEntry->m_caller));
> 
>-    // The tail call was not recursive
>     return false;
> }
> 
>diff --git a/PerformanceTests/ChangeLog b/PerformanceTests/ChangeLog
>index 7140a9b1ba2a0a15eaeb93c2c7b244783f9a30ec..abff849ab10691b2b12a63dda635e5a39a4aea8b 100644
>--- a/PerformanceTests/ChangeLog
>+++ b/PerformanceTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-05-03  Robin Morisset  <rmorisset@apple.com>
>+
>+        Enable tier-up in loops created by recursive tail call optimizations.
>+        https://bugs.webkit.org/show_bug.cgi?id=178389
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Just changed a constant for consistency with TailBench9000/merge-sort.js
>+
>+        * TailBench9000/merge-sort-cps.js:
>+
> 2018-04-30  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         Improve the performance of FontCascadeDescription's effectiveFamilies
>diff --git a/PerformanceTests/TailBench9000/merge-sort-cps.js b/PerformanceTests/TailBench9000/merge-sort-cps.js
>index fff31a6b8d014bb6ea1917fdb2c555b19185f337..710aff5d045db35f8f539fedcf8775901ab8690d 100644
>--- a/PerformanceTests/TailBench9000/merge-sort-cps.js
>+++ b/PerformanceTests/TailBench9000/merge-sort-cps.js
>@@ -131,7 +131,7 @@ function buildArray(length, value)
> let arrays = [
>     buildArray(10, x => x),
>     buildArray(10, x => -x),
>-    buildArray(1000, x => random())
>+    buildArray(10000, x => random())
> ];
> 
> function test(index)

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 178389: 324603 | 339404 | 454036