Changeset 231195 in webkit


Ignore:
Timestamp:
May 1, 2018 8:42:11 AM (6 years ago)
Author:
commit-queue@webkit.org
Message:

Add SetCallee as DFG-Operation
https://bugs.webkit.org/show_bug.cgi?id=184582

Patch by Dominik Infuehr <dinfuehr@igalia.com> on 2018-05-01
Reviewed by Filip Pizlo.

JSTests:

Added test that runs into infinite loop without updating the callee and
therefore emitting SetCallee in DFG for recursive tail calls.

  • stress/closure-recursive-tail-call-infinite-loop.js: Added.

(Foo):
(second):
(first):
(return.closure):
(createClosure):

Source/JavaScriptCore:

For recursive tail calls not only the argument count can change but also the
callee. Add SetCallee to DFG that sets the callee slot in the current call frame.
Also update the callee when optimizing a recursive tail call.
Enable recursive tail call optimization also for closures.

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

  • dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::handleRecursiveTailCall):
(JSC::DFG::ByteCodeParser::handleCallVariant):

  • dfg/DFGClobberize.h:

(JSC::DFG::clobberize):

  • dfg/DFGDoesGC.cpp:

(JSC::DFG::doesGC):

  • dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):

  • dfg/DFGMayExit.cpp:
  • dfg/DFGNodeType.h:
  • dfg/DFGPredictionPropagationPhase.cpp:
  • dfg/DFGSafeToExecute.h:

(JSC::DFG::safeToExecute):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileSetCallee):

  • dfg/DFGSpeculativeJIT.h:
  • dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • ftl/FTLCapabilities.cpp:

(JSC::FTL::canCompile):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileSetCallee):

Location:
trunk
Files:
1 added
17 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r231193 r231195  
     12018-05-01  Dominik Infuehr  <dinfuehr@igalia.com>
     2
     3        Add SetCallee as DFG-Operation
     4        https://bugs.webkit.org/show_bug.cgi?id=184582
     5
     6        Reviewed by Filip Pizlo.
     7
     8        Added test that runs into infinite loop without updating the callee and
     9        therefore emitting SetCallee in DFG for recursive tail calls.
     10
     11        * stress/closure-recursive-tail-call-infinite-loop.js: Added.
     12        (Foo):
     13        (second):
     14        (first):
     15        (return.closure):
     16        (createClosure):
     17
    1182018-04-30  Saam Barati  <sbarati@apple.com>
    219
  • trunk/Source/JavaScriptCore/ChangeLog

    r231194 r231195  
     12018-05-01  Dominik Infuehr  <dinfuehr@igalia.com>
     2
     3        Add SetCallee as DFG-Operation
     4        https://bugs.webkit.org/show_bug.cgi?id=184582
     5
     6        Reviewed by Filip Pizlo.
     7
     8        For recursive tail calls not only the argument count can change but also the
     9        callee. Add SetCallee to DFG that sets the callee slot in the current call frame.
     10        Also update the callee when optimizing a recursive tail call.
     11        Enable recursive tail call optimization also for closures.
     12
     13        * dfg/DFGAbstractInterpreterInlines.h:
     14        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
     15        * dfg/DFGByteCodeParser.cpp:
     16        (JSC::DFG::ByteCodeParser::handleRecursiveTailCall):
     17        (JSC::DFG::ByteCodeParser::handleCallVariant):
     18        * dfg/DFGClobberize.h:
     19        (JSC::DFG::clobberize):
     20        * dfg/DFGDoesGC.cpp:
     21        (JSC::DFG::doesGC):
     22        * dfg/DFGFixupPhase.cpp:
     23        (JSC::DFG::FixupPhase::fixupNode):
     24        * dfg/DFGMayExit.cpp:
     25        * dfg/DFGNodeType.h:
     26        * dfg/DFGPredictionPropagationPhase.cpp:
     27        * dfg/DFGSafeToExecute.h:
     28        (JSC::DFG::safeToExecute):
     29        * dfg/DFGSpeculativeJIT.cpp:
     30        (JSC::DFG::SpeculativeJIT::compileSetCallee):
     31        * dfg/DFGSpeculativeJIT.h:
     32        * dfg/DFGSpeculativeJIT32_64.cpp:
     33        (JSC::DFG::SpeculativeJIT::compile):
     34        * dfg/DFGSpeculativeJIT64.cpp:
     35        (JSC::DFG::SpeculativeJIT::compile):
     36        * ftl/FTLCapabilities.cpp:
     37        (JSC::FTL::canCompile):
     38        * ftl/FTLLowerDFGToB3.cpp:
     39        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
     40        (JSC::FTL::DFG::LowerDFGToB3::compileSetCallee):
     41
    1422018-05-01  Oleksandr Skachkov  <gskachkov@gmail.com>
    243
  • trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

    r231145 r231195  
    24492449        break;
    24502450
     2451    case SetCallee:
    24512452    case SetArgumentCountIncludingThis:
    24522453        break;
  • trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

    r230929 r231195  
    157157    Node* getArgumentCount();
    158158    template<typename ChecksFunctor>
    159     bool handleRecursiveTailCall(CallVariant, int registerOffset, int argumentCountIncludingThis, const ChecksFunctor& emitFunctionCheckIfNeeded);
     159    bool handleRecursiveTailCall(Node* callTargetNode, CallVariant, int registerOffset, int argumentCountIncludingThis, const ChecksFunctor& emitFunctionCheckIfNeeded);
    160160    unsigned inliningCost(CallVariant, int argumentCountIncludingThis, InlineCallFrame::Kind); // Return UINT_MAX if it's not an inlining candidate. By convention, intrinsics have a cost of 1.
    161161    // Handle inlining. Return true if it succeeded, false if we need to plant a call.
     
    13571357
    13581358template<typename ChecksFunctor>
    1359 bool ByteCodeParser::handleRecursiveTailCall(CallVariant callVariant, int registerOffset, int argumentCountIncludingThis, const ChecksFunctor& emitFunctionCheckIfNeeded)
     1359bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant callVariant, int registerOffset, int argumentCountIncludingThis, const ChecksFunctor& emitFunctionCheckIfNeeded)
    13601360{
    13611361    if (UNLIKELY(!Options::optimizeRecursiveTailCalls()))
    1362         return false;
    1363 
    1364     // Currently we cannot do this optimisation for closures. The problem is that "emitFunctionChecks" which we use later is too coarse, only checking the executable
    1365     // and not the value of captured variables.
    1366     if (callVariant.isClosureCall())
    13671362        return false;
    13681363
     
    13861381        }
    13871382
     1383        // If an InlineCallFrame is not a closure, it was optimized using a constant callee.
     1384        // Check if this is the same callee that we try to inline here.
     1385        if (stackEntry->m_inlineCallFrame && !stackEntry->m_inlineCallFrame->isClosureCall) {
     1386            if (stackEntry->m_inlineCallFrame->calleeConstant() != callVariant.function())
     1387                continue;
     1388        }
     1389
    13881390        // We must add some check that the profiling information was correct and the target of this call is what we thought.
    13891391        emitFunctionCheckIfNeeded();
    13901392        // We flush everything, as if we were in the backedge of a loop (see treatment of op_jmp in parseBlock).
    13911393        flushForTerminal();
     1394
     1395        // We must set the callee to the right value
     1396        if (stackEntry->m_inlineCallFrame) {
     1397            if (stackEntry->m_inlineCallFrame->isClosureCall)
     1398                setDirect(stackEntry->remapOperand(VirtualRegister(CallFrameSlot::callee)), callTargetNode, NormalSet);
     1399        } else
     1400            addToGraph(SetCallee, callTargetNode);
    13921401
    13931402        // We must set the arguments to the right values
     
    16841693    };
    16851694
    1686     if (kind == InlineCallFrame::TailCall && ByteCodeParser::handleRecursiveTailCall(callee, registerOffset, argumentCountIncludingThis, insertChecksWithAccounting)) {
     1695    if (kind == InlineCallFrame::TailCall && ByteCodeParser::handleRecursiveTailCall(callTargetNode, callee, registerOffset, argumentCountIncludingThis, insertChecksWithAccounting)) {
    16871696        RELEASE_ASSERT(didInsertChecks);
    16881697        return CallOptimizationResult::OptimizedToJump;
  • trunk/Source/JavaScriptCore/dfg/DFGClobberize.h

    r231145 r231195  
    696696        def(HeapLocation(StackLoc, AbstractHeap(Stack, CallFrameSlot::callee)), LazyNode(node));
    697697        return;
     698
     699    case SetCallee:
     700        write(AbstractHeap(Stack, CallFrameSlot::callee));
     701        return;
    698702       
    699703    case GetArgumentCountIncludingThis: {
  • trunk/Source/JavaScriptCore/dfg/DFGDoesGC.cpp

    r230376 r231195  
    5252    case IdentityWithProfile:
    5353    case GetCallee:
     54    case SetCallee:
    5455    case GetArgumentCountIncludingThis:
    5556    case SetArgumentCountIncludingThis:
  • trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

    r230516 r231195  
    21332133                break;
    21342134            }
     2135            break;
     2136
     2137        case SetCallee:
     2138            fixEdge<CellUse>(node->child1());
    21352139            break;
    21362140
  • trunk/Source/JavaScriptCore/dfg/DFGMayExit.cpp

    r229514 r231195  
    7575    case GetStack:
    7676    case GetCallee:
     77    case SetCallee:
    7778    case GetArgumentCountIncludingThis:
    7879    case SetArgumentCountIncludingThis:
  • trunk/Source/JavaScriptCore/dfg/DFGNodeType.h

    r230964 r231195  
    5555    macro(CreateThis, NodeResultJS) /* Note this is not MustGenerate since we're returning it anyway. */ \
    5656    macro(GetCallee, NodeResultJS) \
     57    macro(SetCallee, NodeMustGenerate) \
    5758    macro(GetArgumentCountIncludingThis, NodeResultInt32) \
    5859    macro(SetArgumentCountIncludingThis, NodeMustGenerate) \
  • trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp

    r230516 r231195  
    777777        }
    778778
     779        case SetCallee:
    779780        case SetArgumentCountIncludingThis:
    780781            break;
  • trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h

    r230516 r231195  
    173173    case CreateThis:
    174174    case GetCallee:
     175    case SetCallee:
    175176    case GetArgumentCountIncludingThis:
    176177    case SetArgumentCountIncludingThis:
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r231129 r231195  
    1186711867}
    1186811868
     11869void SpeculativeJIT::compileSetCallee(Node* node)
     11870{
     11871    SpeculateCellOperand callee(this, node->child1());
     11872    m_jit.storeCell(callee.gpr(), JITCompiler::payloadFor(CallFrameSlot::callee));
     11873    noResult(node);
     11874}
     11875
    1186911876void SpeculativeJIT::compileGetArgumentCountIncludingThis(Node* node)
    1187011877{
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h

    r230748 r231195  
    14601460    void compileGetSetter(Node*);
    14611461    void compileGetCallee(Node*);
     1462    void compileSetCallee(Node*);
    14621463    void compileGetArgumentCountIncludingThis(Node*);
    14631464    void compileSetArgumentCountIncludingThis(Node*);
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp

    r230626 r231195  
    32543254    case GetCallee: {
    32553255        compileGetCallee(node);
     3256        break;
     3257    }
     3258
     3259    case SetCallee: {
     3260        compileSetCallee(node);
    32563261        break;
    32573262    }
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

    r230748 r231195  
    34753475    case GetCallee: {
    34763476        compileGetCallee(node);
     3477        break;
     3478    }
     3479
     3480    case SetCallee: {
     3481        compileSetCallee(node);
    34773482        break;
    34783483    }
  • trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp

    r230516 r231195  
    183183    case GetScope:
    184184    case GetCallee:
     185    case SetCallee:
    185186    case GetArgumentCountIncludingThis:
    186187    case SetArgumentCountIncludingThis:
  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r231129 r231195  
    925925            compileGetCallee();
    926926            break;
     927        case SetCallee:
     928            compileSetCallee();
     929            break;
    927930        case GetArgumentCountIncludingThis:
    928931            compileGetArgumentCountIncludingThis();
     
    66266629    {
    66276630        setJSValue(m_out.loadPtr(addressFor(CallFrameSlot::callee)));
     6631    }
     6632
     6633    void compileSetCallee()
     6634    {
     6635        auto callee = lowCell(m_node->child1());
     6636        m_out.storePtr(callee, payloadFor(CallFrameSlot::callee));
    66286637    }
    66296638   
Note: See TracChangeset for help on using the changeset viewer.