- Revision
- 280847
- Author
- [email protected]
- Date
- 2021-08-10 10:08:11 -0700 (Tue, 10 Aug 2021)
Log Message
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/_javascript_Core:
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
Modified Paths
Added Paths
Diff
Modified: branches/safari-611.3.10.0-branch/JSTests/ChangeLog (280846 => 280847)
--- 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)
@@ -1,3 +1,71 @@
+2021-08-10 Russell Epstein <[email protected]>
+
+ 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/_javascript_Core:
+
+ 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 <[email protected]>
+
+ 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):
+
2021-06-15 Alan Coon <[email protected]>
Cherry-pick r278819. rdar://problem/79355258
Added: branches/safari-611.3.10.0-branch/JSTests/stress/dfg-arity-fixup-uses-callers-exit-origin.js (0 => 280847)
--- 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)
@@ -0,0 +1,54 @@
+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();
Modified: branches/safari-611.3.10.0-branch/Source/_javascript_Core/ChangeLog (280846 => 280847)
--- branches/safari-611.3.10.0-branch/Source/_javascript_Core/ChangeLog 2021-08-10 16:56:35 UTC (rev 280846)
+++ branches/safari-611.3.10.0-branch/Source/_javascript_Core/ChangeLog 2021-08-10 17:08:11 UTC (rev 280847)
@@ -1,3 +1,89 @@
+2021-08-10 Russell Epstein <[email protected]>
+
+ 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/_javascript_Core:
+
+ 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 <[email protected]>
+
+ 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):
+
2021-06-15 Alan Coon <[email protected]>
Cherry-pick r278819. rdar://problem/79355258
Modified: branches/safari-611.3.10.0-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (280846 => 280847)
--- branches/safari-611.3.10.0-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2021-08-10 16:56:35 UTC (rev 280846)
+++ branches/safari-611.3.10.0-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2021-08-10 17:08:11 UTC (rev 280847)
@@ -746,16 +746,9 @@
NodeOrigin currentNodeOrigin()
{
- CodeOrigin semantic;
- CodeOrigin forExit;
+ CodeOrigin semantic = m_currentSemanticOrigin.isSet() ? m_currentSemanticOrigin : currentCodeOrigin();
+ CodeOrigin forExit = m_currentExitOrigin.isSet() ? m_currentExitOrigin : currentCodeOrigin();
- if (m_currentSemanticOrigin.isSet())
- semantic = m_currentSemanticOrigin;
- else
- semantic = currentCodeOrigin();
-
- forExit = currentCodeOrigin();
-
return NodeOrigin(semantic, forExit, m_exitOK);
}
@@ -1144,6 +1137,8 @@
BytecodeIndex m_currentIndex;
// The semantic origin of the current node if different from the current Index.
CodeOrigin m_currentSemanticOrigin;
+ // The exit origin of the current node if different from the current Index.
+ CodeOrigin m_currentExitOrigin;
// True if it's OK to OSR exit right now.
bool m_exitOK { false };
@@ -1711,6 +1706,31 @@
calleeVariable->mergeShouldNeverUnbox(true);
}
+ // 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();
+
InlineStackEntry* callerStackTop = m_inlineStackTop;
InlineStackEntry inlineStackEntry(this, codeBlock, codeBlock, callee.function(), result,
inlineCallFrameStart.virtualRegister(), argumentCountIncludingThis, kind, continuationBlock);
@@ -1814,6 +1834,8 @@
// our callee's frame. We emit an ExitOK below.
}
+ m_currentExitOrigin = oldExitOrigin;
+
// At this point, it's again OK to OSR exit.
m_exitOK = true;
addToGraph(ExitOK);
