<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[280847] branches/safari-611.3.10.0-branch</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/280847">280847</a></dd>
<dt>Author</dt> <dd>repstein@apple.com</dd>
<dt>Date</dt> <dd>2021-08-10 10:08:11 -0700 (Tue, 10 Aug 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>Cherry-pick <a href="http://trac.webkit.org/projects/webkit/changeset/275472">r275472</a>. rdar://problem/81710596

    DFG arity fixup nodes should exit to the caller's call opcode
    https://bugs.webkit.org/show_bug.cgi?id=223278

    Reviewed by Saam Barati.

    JSTests:

    * stress/dfg-arity-fixup-uses-callers-exit-origin.js: Added.
    (main.v22):
    (main.v30):
    (main.try.v40):
    (main.try.v47):
    (main.try.v56):
    (main.):
    (main):

    Source/JavaScriptCore:

    Right now when we do arity fixup in the DFG we model it in the
    same way that it executes, which means all the nodes are part of
    the callee. Unfortunately, this causes PhantomInsertionPhase to
    think those nodes could be replacing previously defined
    VirtualRegisters as they are part of the callee's header (always
    alive). When PhantomInsertionPhase then inserts a Phantom it will
    put that node in the caller's frame as that's the first ExitOK
    node. The caller however may have no knowledge of that
    VirtualRegister though. For example:

    --> foo: loc10 is a local in foo.
        ...
        1: MovHint(loc10)
        2: SetLocal(loc10)
    <-- foo // loc10 ten is now out of scope for the InlineCallFrame of the caller.
    ...
    // Phantom will be inserted here refering to loc10, which doesn't make sense.
    --> bar // loc10 is an argument to bar and needs arity fixup.
        ... // All of these nodes are ExitInvalid
        3: MovHint(loc10, ExitInvalid)
        4: SetLocal(loc10, ExitInvalid)
        ...

    * dfg/DFGByteCodeParser.cpp:
    (JSC::DFG::ByteCodeParser::currentNodeOrigin):
    (JSC::DFG::ByteCodeParser::inlineCall):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275472 268f45cc-cd09-0410-ab3c-d52691b4dbfc</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#branchessafari6113100branchJSTestsChangeLog">branches/safari-611.3.10.0-branch/JSTests/ChangeLog</a></li>
<li><a href="#branchessafari6113100branchSourceJavaScriptCoreChangeLog">branches/safari-611.3.10.0-branch/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#branchessafari6113100branchSourceJavaScriptCoredfgDFGByteCodeParsercpp">branches/safari-611.3.10.0-branch/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#branchessafari6113100branchJSTestsstressdfgarityfixupusescallersexitoriginjs">branches/safari-611.3.10.0-branch/JSTests/stress/dfg-arity-fixup-uses-callers-exit-origin.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="branchessafari6113100branchJSTestsChangeLog"></a>
<div class="modfile"><h4>Modified: branches/safari-611.3.10.0-branch/JSTests/ChangeLog (280846 => 280847)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-611.3.10.0-branch/JSTests/ChangeLog      2021-08-10 16:56:35 UTC (rev 280846)
+++ branches/safari-611.3.10.0-branch/JSTests/ChangeLog 2021-08-10 17:08:11 UTC (rev 280847)
</span><span class="lines">@@ -1,3 +1,71 @@
</span><ins>+2021-08-10  Russell Epstein  <repstein@apple.com>
+
+        Cherry-pick r275472. rdar://problem/81710596
+
+    DFG arity fixup nodes should exit to the caller's call opcode
+    https://bugs.webkit.org/show_bug.cgi?id=223278
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * stress/dfg-arity-fixup-uses-callers-exit-origin.js: Added.
+    (main.v22):
+    (main.v30):
+    (main.try.v40):
+    (main.try.v47):
+    (main.try.v56):
+    (main.):
+    (main):
+    
+    Source/JavaScriptCore:
+    
+    Right now when we do arity fixup in the DFG we model it in the
+    same way that it executes, which means all the nodes are part of
+    the callee. Unfortunately, this causes PhantomInsertionPhase to
+    think those nodes could be replacing previously defined
+    VirtualRegisters as they are part of the callee's header (always
+    alive). When PhantomInsertionPhase then inserts a Phantom it will
+    put that node in the caller's frame as that's the first ExitOK
+    node. The caller however may have no knowledge of that
+    VirtualRegister though. For example:
+    
+    --> foo: loc10 is a local in foo.
+        ...
+        1: MovHint(loc10)
+        2: SetLocal(loc10)
+    <-- foo // loc10 ten is now out of scope for the InlineCallFrame of the caller.
+    ...
+    // Phantom will be inserted here refering to loc10, which doesn't make sense.
+    --> bar // loc10 is an argument to bar and needs arity fixup.
+        ... // All of these nodes are ExitInvalid
+        3: MovHint(loc10, ExitInvalid)
+        4: SetLocal(loc10, ExitInvalid)
+        ...
+    
+    * dfg/DFGByteCodeParser.cpp:
+    (JSC::DFG::ByteCodeParser::currentNodeOrigin):
+    (JSC::DFG::ByteCodeParser::inlineCall):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275472 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-04-05  Keith Miller  <keith_miller@apple.com>
+
+            DFG arity fixup nodes should exit to the caller's call opcode
+            https://bugs.webkit.org/show_bug.cgi?id=223278
+
+            Reviewed by Saam Barati.
+
+            * stress/dfg-arity-fixup-uses-callers-exit-origin.js: Added.
+            (main.v22):
+            (main.v30):
+            (main.try.v40):
+            (main.try.v47):
+            (main.try.v56):
+            (main.):
+            (main):
+
</ins><span class="cx"> 2021-06-15  Alan Coon  <alancoon@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Cherry-pick r278819. rdar://problem/79355258
</span></span></pre></div>
<a id="branchessafari6113100branchJSTestsstressdfgarityfixupusescallersexitoriginjs"></a>
<div class="addfile"><h4>Added: branches/safari-611.3.10.0-branch/JSTests/stress/dfg-arity-fixup-uses-callers-exit-origin.js (0 => 280847)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-611.3.10.0-branch/JSTests/stress/dfg-arity-fixup-uses-callers-exit-origin.js                             (rev 0)
+++ branches/safari-611.3.10.0-branch/JSTests/stress/dfg-arity-fixup-uses-callers-exit-origin.js        2021-08-10 17:08:11 UTC (rev 280847)
</span><span class="lines">@@ -0,0 +1,54 @@
</span><ins>+function main() {
+const v12 = [1337,1337];
+const v13 = [1337,v12,v12,0];
+for (let v14 = 0; v14 < 1000; v14++) {
+    function v15(v16,v17) {
+        const v18 = v14 + 127;
+        const v19 = String();
+        const v20 = String.fromCharCode();
+        const v21 = v13.shift();
+        function v22() {
+            const v23 = arguments;
+        }
+        const v24 = Object();
+        const v25 = {};
+        const v26 = v22(v25);
+        const v27 = [-903931.176976766,v20,null,null,-903931.176976766];
+        function v30() {
+        }
+        const v31 = {ownKeys:v30};
+        const v32 = {};
+        const v33 = new Proxy(v32,v31);
+        Function.__proto__ = v33;
+        const v34 = v27.join();
+        try {
+            const v35 = Function();
+            const v36 = v35();
+            for (let v37 = 0; v37 < 127; v37++) {
+                const v38 = isFinite();
+                const v39 = isFinite;
+                function v40(v41,v42,v43) {
+                }
+                const v44 = 1337;
+                const v45 = undefined;
+                const v46 = "function(){}";
+                function* v47(v48,v49,v50,v51,v52) {
+                }
+                const v53 = charAt;
+                function v56(v57,v58,v59,v60,v61) {
+                    const v62 = v36(v35,v37);
+                }
+                for (let v64 = 0; v64 >= 100000; v64++) {
+                }
+                const v65 = 10000;
+                const v66 = v38[4];
+            }
+        } catch(v67) {
+        }
+    }
+    const v68 = v15();
+}
+}
+noDFG(main);
+noFTL(main);
+main();
</ins></span></pre></div>
<a id="branchessafari6113100branchSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: branches/safari-611.3.10.0-branch/Source/JavaScriptCore/ChangeLog (280846 => 280847)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-611.3.10.0-branch/Source/JavaScriptCore/ChangeLog        2021-08-10 16:56:35 UTC (rev 280846)
+++ branches/safari-611.3.10.0-branch/Source/JavaScriptCore/ChangeLog   2021-08-10 17:08:11 UTC (rev 280847)
</span><span class="lines">@@ -1,3 +1,89 @@
</span><ins>+2021-08-10  Russell Epstein  <repstein@apple.com>
+
+        Cherry-pick r275472. rdar://problem/81710596
+
+    DFG arity fixup nodes should exit to the caller's call opcode
+    https://bugs.webkit.org/show_bug.cgi?id=223278
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * stress/dfg-arity-fixup-uses-callers-exit-origin.js: Added.
+    (main.v22):
+    (main.v30):
+    (main.try.v40):
+    (main.try.v47):
+    (main.try.v56):
+    (main.):
+    (main):
+    
+    Source/JavaScriptCore:
+    
+    Right now when we do arity fixup in the DFG we model it in the
+    same way that it executes, which means all the nodes are part of
+    the callee. Unfortunately, this causes PhantomInsertionPhase to
+    think those nodes could be replacing previously defined
+    VirtualRegisters as they are part of the callee's header (always
+    alive). When PhantomInsertionPhase then inserts a Phantom it will
+    put that node in the caller's frame as that's the first ExitOK
+    node. The caller however may have no knowledge of that
+    VirtualRegister though. For example:
+    
+    --> foo: loc10 is a local in foo.
+        ...
+        1: MovHint(loc10)
+        2: SetLocal(loc10)
+    <-- foo // loc10 ten is now out of scope for the InlineCallFrame of the caller.
+    ...
+    // Phantom will be inserted here refering to loc10, which doesn't make sense.
+    --> bar // loc10 is an argument to bar and needs arity fixup.
+        ... // All of these nodes are ExitInvalid
+        3: MovHint(loc10, ExitInvalid)
+        4: SetLocal(loc10, ExitInvalid)
+        ...
+    
+    * dfg/DFGByteCodeParser.cpp:
+    (JSC::DFG::ByteCodeParser::currentNodeOrigin):
+    (JSC::DFG::ByteCodeParser::inlineCall):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275472 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-04-05  Keith Miller  <keith_miller@apple.com>
+
+            DFG arity fixup nodes should exit to the caller's call opcode
+            https://bugs.webkit.org/show_bug.cgi?id=223278
+
+            Reviewed by Saam Barati.
+
+            Right now when we do arity fixup in the DFG we model it in the
+            same way that it executes, which means all the nodes are part of
+            the callee. Unfortunately, this causes PhantomInsertionPhase to
+            think those nodes could be replacing previously defined
+            VirtualRegisters as they are part of the callee's header (always
+            alive). When PhantomInsertionPhase then inserts a Phantom it will
+            put that node in the caller's frame as that's the first ExitOK
+            node. The caller however may have no knowledge of that
+            VirtualRegister though. For example:
+
+            --> foo: loc10 is a local in foo.
+                ...
+                1: MovHint(loc10)
+                2: SetLocal(loc10)
+            <-- foo // loc10 ten is now out of scope for the InlineCallFrame of the caller.
+            ...
+            // Phantom will be inserted here refering to loc10, which doesn't make sense.
+            --> bar // loc10 is an argument to bar and needs arity fixup.
+                ... // All of these nodes are ExitInvalid
+                3: MovHint(loc10, ExitInvalid)
+                4: SetLocal(loc10, ExitInvalid)
+                ...
+
+            * dfg/DFGByteCodeParser.cpp:
+            (JSC::DFG::ByteCodeParser::currentNodeOrigin):
+            (JSC::DFG::ByteCodeParser::inlineCall):
+
</ins><span class="cx"> 2021-06-15  Alan Coon  <alancoon@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Cherry-pick r278819. rdar://problem/79355258
</span></span></pre></div>
<a id="branchessafari6113100branchSourceJavaScriptCoredfgDFGByteCodeParsercpp"></a>
<div class="modfile"><h4>Modified: branches/safari-611.3.10.0-branch/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (280846 => 280847)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-611.3.10.0-branch/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp        2021-08-10 16:56:35 UTC (rev 280846)
+++ branches/safari-611.3.10.0-branch/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp   2021-08-10 17:08:11 UTC (rev 280847)
</span><span class="lines">@@ -746,16 +746,9 @@
</span><span class="cx"> 
</span><span class="cx">     NodeOrigin currentNodeOrigin()
</span><span class="cx">     {
</span><del>-        CodeOrigin semantic;
-        CodeOrigin forExit;
</del><ins>+        CodeOrigin semantic = m_currentSemanticOrigin.isSet() ? m_currentSemanticOrigin : currentCodeOrigin();
+        CodeOrigin forExit = m_currentExitOrigin.isSet() ? m_currentExitOrigin : currentCodeOrigin();
</ins><span class="cx"> 
</span><del>-        if (m_currentSemanticOrigin.isSet())
-            semantic = m_currentSemanticOrigin;
-        else
-            semantic = currentCodeOrigin();
-
-        forExit = currentCodeOrigin();
-
</del><span class="cx">         return NodeOrigin(semantic, forExit, m_exitOK);
</span><span class="cx">     }
</span><span class="cx">     
</span><span class="lines">@@ -1144,6 +1137,8 @@
</span><span class="cx">     BytecodeIndex m_currentIndex;
</span><span class="cx">     // The semantic origin of the current node if different from the current Index.
</span><span class="cx">     CodeOrigin m_currentSemanticOrigin;
</span><ins>+    // The exit origin of the current node if different from the current Index.
+    CodeOrigin m_currentExitOrigin;
</ins><span class="cx">     // True if it's OK to OSR exit right now.
</span><span class="cx">     bool m_exitOK { false };
</span><span class="cx"> 
</span><span class="lines">@@ -1711,6 +1706,31 @@
</span><span class="cx">         calleeVariable->mergeShouldNeverUnbox(true);
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    // We want to claim the exit origin for the arity fixup nodes to be in the caller rather than the callee because
+    // otherwise phantom insertion phase will think the virtual registers in the callee's header have been alive from the last
+    // time they were set. For example:
+
+    // --> foo: loc10 is a local in foo.
+    //    ...
+    //    1: MovHint(loc10)
+    //    2: SetLocal(loc10)
+    // <-- foo: loc10 ten is now out of scope for the InlineCallFrame of the caller.
+    // ...
+    // --> bar: loc10 is an argument to bar and needs arity fixup.
+    //    ... // All of these nodes are ExitInvalid
+    //    3: MovHint(loc10, ExitInvalid)
+    //    4: SetLocal(loc10, ExitInvalid)
+    //    ...
+
+    // In this example phantom insertion phase will think @3 is always alive because it's in the header of bar. So,
+    // it will think we are about to kill the old value, as loc10 is in the header of bar and therefore always live, and
+    // thus need a Phantom. That Phantom, however, may be inserted  into the caller's NodeOrigin (all the nodes in bar
+    // before @3 are ExitInvalid), which doesn't know about loc10. If we move all of the arity fixup nodes into the
+    // caller's exit origin, forAllKilledOperands, which is how phantom insertion phase decides where phantoms are needed,
+    // will no longer say loc10 is always alive.
+    CodeOrigin oldExitOrigin = m_currentExitOrigin;
+    m_currentExitOrigin = currentCodeOrigin();
+
</ins><span class="cx">     InlineStackEntry* callerStackTop = m_inlineStackTop;
</span><span class="cx">     InlineStackEntry inlineStackEntry(this, codeBlock, codeBlock, callee.function(), result,
</span><span class="cx">         inlineCallFrameStart.virtualRegister(), argumentCountIncludingThis, kind, continuationBlock);
</span><span class="lines">@@ -1814,6 +1834,8 @@
</span><span class="cx">         // our callee's frame. We emit an ExitOK below.
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    m_currentExitOrigin = oldExitOrigin;
+
</ins><span class="cx">     // At this point, it's again OK to OSR exit.
</span><span class="cx">     m_exitOK = true;
</span><span class="cx">     addToGraph(ExitOK);
</span></span></pre>
</div>
</div>

</body>
</html>