⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 280847 in webkit


Ignore:
Timestamp:
Aug 10, 2021, 10:08:11 AM (5 years ago)
Author:
Russell Epstein
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/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

Location:
branches/safari-611.3.10.0-branch
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-611.3.10.0-branch/JSTests/ChangeLog

    r278904 r280847  
     12021-08-10  Russell Epstein  <repstein@apple.com>
     2
     3        Cherry-pick r275472. rdar://problem/81710596
     4
     5    DFG arity fixup nodes should exit to the caller's call opcode
     6    https://bugs.webkit.org/show_bug.cgi?id=223278
     7   
     8    Reviewed by Saam Barati.
     9   
     10    JSTests:
     11   
     12    * stress/dfg-arity-fixup-uses-callers-exit-origin.js: Added.
     13    (main.v22):
     14    (main.v30):
     15    (main.try.v40):
     16    (main.try.v47):
     17    (main.try.v56):
     18    (main.):
     19    (main):
     20   
     21    Source/JavaScriptCore:
     22   
     23    Right now when we do arity fixup in the DFG we model it in the
     24    same way that it executes, which means all the nodes are part of
     25    the callee. Unfortunately, this causes PhantomInsertionPhase to
     26    think those nodes could be replacing previously defined
     27    VirtualRegisters as they are part of the callee's header (always
     28    alive). When PhantomInsertionPhase then inserts a Phantom it will
     29    put that node in the caller's frame as that's the first ExitOK
     30    node. The caller however may have no knowledge of that
     31    VirtualRegister though. For example:
     32   
     33    --> foo: loc10 is a local in foo.
     34        ...
     35        1: MovHint(loc10)
     36        2: SetLocal(loc10)
     37    <-- foo // loc10 ten is now out of scope for the InlineCallFrame of the caller.
     38    ...
     39    // Phantom will be inserted here refering to loc10, which doesn't make sense.
     40    --> bar // loc10 is an argument to bar and needs arity fixup.
     41        ... // All of these nodes are ExitInvalid
     42        3: MovHint(loc10, ExitInvalid)
     43        4: SetLocal(loc10, ExitInvalid)
     44        ...
     45   
     46    * dfg/DFGByteCodeParser.cpp:
     47    (JSC::DFG::ByteCodeParser::currentNodeOrigin):
     48    (JSC::DFG::ByteCodeParser::inlineCall):
     49   
     50   
     51    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275472 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     52
     53    2021-04-05  Keith Miller  <keith_miller@apple.com>
     54
     55            DFG arity fixup nodes should exit to the caller's call opcode
     56            https://bugs.webkit.org/show_bug.cgi?id=223278
     57
     58            Reviewed by Saam Barati.
     59
     60            * stress/dfg-arity-fixup-uses-callers-exit-origin.js: Added.
     61            (main.v22):
     62            (main.v30):
     63            (main.try.v40):
     64            (main.try.v47):
     65            (main.try.v56):
     66            (main.):
     67            (main):
     68
    1692021-06-15  Alan Coon  <alancoon@apple.com>
    270

  • branches/safari-611.3.10.0-branch/Source/JavaScriptCore/ChangeLog

    r278904 r280847  
     12021-08-10  Russell Epstein  <repstein@apple.com>
     2
     3        Cherry-pick r275472. rdar://problem/81710596
     4
     5    DFG arity fixup nodes should exit to the caller's call opcode
     6    https://bugs.webkit.org/show_bug.cgi?id=223278
     7   
     8    Reviewed by Saam Barati.
     9   
     10    JSTests:
     11   
     12    * stress/dfg-arity-fixup-uses-callers-exit-origin.js: Added.
     13    (main.v22):
     14    (main.v30):
     15    (main.try.v40):
     16    (main.try.v47):
     17    (main.try.v56):
     18    (main.):
     19    (main):
     20   
     21    Source/JavaScriptCore:
     22   
     23    Right now when we do arity fixup in the DFG we model it in the
     24    same way that it executes, which means all the nodes are part of
     25    the callee. Unfortunately, this causes PhantomInsertionPhase to
     26    think those nodes could be replacing previously defined
     27    VirtualRegisters as they are part of the callee's header (always
     28    alive). When PhantomInsertionPhase then inserts a Phantom it will
     29    put that node in the caller's frame as that's the first ExitOK
     30    node. The caller however may have no knowledge of that
     31    VirtualRegister though. For example:
     32   
     33    --> foo: loc10 is a local in foo.
     34        ...
     35        1: MovHint(loc10)
     36        2: SetLocal(loc10)
     37    <-- foo // loc10 ten is now out of scope for the InlineCallFrame of the caller.
     38    ...
     39    // Phantom will be inserted here refering to loc10, which doesn't make sense.
     40    --> bar // loc10 is an argument to bar and needs arity fixup.
     41        ... // All of these nodes are ExitInvalid
     42        3: MovHint(loc10, ExitInvalid)
     43        4: SetLocal(loc10, ExitInvalid)
     44        ...
     45   
     46    * dfg/DFGByteCodeParser.cpp:
     47    (JSC::DFG::ByteCodeParser::currentNodeOrigin):
     48    (JSC::DFG::ByteCodeParser::inlineCall):
     49   
     50   
     51    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275472 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     52
     53    2021-04-05  Keith Miller  <keith_miller@apple.com>
     54
     55            DFG arity fixup nodes should exit to the caller's call opcode
     56            https://bugs.webkit.org/show_bug.cgi?id=223278
     57
     58            Reviewed by Saam Barati.
     59
     60            Right now when we do arity fixup in the DFG we model it in the
     61            same way that it executes, which means all the nodes are part of
     62            the callee. Unfortunately, this causes PhantomInsertionPhase to
     63            think those nodes could be replacing previously defined
     64            VirtualRegisters as they are part of the callee's header (always
     65            alive). When PhantomInsertionPhase then inserts a Phantom it will
     66            put that node in the caller's frame as that's the first ExitOK
     67            node. The caller however may have no knowledge of that
     68            VirtualRegister though. For example:
     69
     70            --> foo: loc10 is a local in foo.
     71                ...
     72                1: MovHint(loc10)
     73                2: SetLocal(loc10)
     74            <-- foo // loc10 ten is now out of scope for the InlineCallFrame of the caller.
     75            ...
     76            // Phantom will be inserted here refering to loc10, which doesn't make sense.
     77            --> bar // loc10 is an argument to bar and needs arity fixup.
     78                ... // All of these nodes are ExitInvalid
     79                3: MovHint(loc10, ExitInvalid)
     80                4: SetLocal(loc10, ExitInvalid)
     81                ...
     82
     83            * dfg/DFGByteCodeParser.cpp:
     84            (JSC::DFG::ByteCodeParser::currentNodeOrigin):
     85            (JSC::DFG::ByteCodeParser::inlineCall):
     86
    1872021-06-15  Alan Coon  <alancoon@apple.com>
    288

  • branches/safari-611.3.10.0-branch/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

    r276664 r280847  
    747747    NodeOrigin currentNodeOrigin()
    748748    {
    749         CodeOrigin semantic;
    750         CodeOrigin forExit;
    751 
    752         if (m_currentSemanticOrigin.isSet())
    753             semantic = m_currentSemanticOrigin;
    754         else
    755             semantic = currentCodeOrigin();
    756 
    757         forExit = currentCodeOrigin();
     749        CodeOrigin semantic = m_currentSemanticOrigin.isSet() ? m_currentSemanticOrigin : currentCodeOrigin();
     750        CodeOrigin forExit = m_currentExitOrigin.isSet() ? m_currentExitOrigin : currentCodeOrigin();
    758751
    759752        return NodeOrigin(semantic, forExit, m_exitOK);
    … …  
    11451138    // The semantic origin of the current node if different from the current Index.
    11461139    CodeOrigin m_currentSemanticOrigin;
     1140    // The exit origin of the current node if different from the current Index.
     1141    CodeOrigin m_currentExitOrigin;
    11471142    // True if it's OK to OSR exit right now.
    11481143    bool m_exitOK { false };
    … …  
    17111706        calleeVariable->mergeShouldNeverUnbox(true);
    17121707    }
     1708
     1709    // We want to claim the exit origin for the arity fixup nodes to be in the caller rather than the callee because
     1710    // otherwise phantom insertion phase will think the virtual registers in the callee's header have been alive from the last
     1711    // time they were set. For example:
     1712
     1713    // --> foo: loc10 is a local in foo.
     1714    //    ...
     1715    //    1: MovHint(loc10)
     1716    //    2: SetLocal(loc10)
     1717    // <-- foo: loc10 ten is now out of scope for the InlineCallFrame of the caller.
     1718    // ...
     1719    // --> bar: loc10 is an argument to bar and needs arity fixup.
     1720    //    ... // All of these nodes are ExitInvalid
     1721    //    3: MovHint(loc10, ExitInvalid)
     1722    //    4: SetLocal(loc10, ExitInvalid)
     1723    //    ...
     1724
     1725    // In this example phantom insertion phase will think @3 is always alive because it's in the header of bar. So,
     1726    // it will think we are about to kill the old value, as loc10 is in the header of bar and therefore always live, and
     1727    // thus need a Phantom. That Phantom, however, may be inserted  into the caller's NodeOrigin (all the nodes in bar
     1728    // before @3 are ExitInvalid), which doesn't know about loc10. If we move all of the arity fixup nodes into the
     1729    // caller's exit origin, forAllKilledOperands, which is how phantom insertion phase decides where phantoms are needed,
     1730    // will no longer say loc10 is always alive.
     1731    CodeOrigin oldExitOrigin = m_currentExitOrigin;
     1732    m_currentExitOrigin = currentCodeOrigin();
    17131733
    17141734    InlineStackEntry* callerStackTop = m_inlineStackTop;
    … …  
    18141834        // our callee's frame. We emit an ExitOK below.
    18151835    }
     1836
     1837    m_currentExitOrigin = oldExitOrigin;
    18161838
    18171839    // At this point, it's again OK to OSR exit.
Note: See TracChangeset for help on using the changeset viewer.