Changeset 160493 in webkit


Ignore:
Timestamp:
Dec 12, 2013 10:38:39 AM (10 years ago)
Author:
fpizlo@apple.com
Message:

ARM64: Hang running pdfjs test, suspect DFG generated code for "in"
https://bugs.webkit.org/show_bug.cgi?id=124727
<rdar://problem/15566923>

Reviewed by Michael Saboff.

Get rid of In's hackish use of StructureStubInfo. Previously it was using hotPathBegin,
and it was the only IC that used that field, which was wasteful. Moreover, it used it
to store two separate locations: the label for patching the jump and the label right
after the jump. The code was relying on those two being the same label, which is true
on X86 and some other platforms, but it isn't true on ARM64.

This gets rid of hotPathBegin and makes In express those two locations as offsets from
the callReturnLocation, which is analogous to what the other IC's do.

This fixes a bug where any successful In patching would result in a trivially infinite
loop - and hence a hang - on ARM64.

  • bytecode/StructureStubInfo.h:
  • dfg/DFGJITCompiler.cpp:

(JSC::DFG::JITCompiler::link):

  • dfg/DFGJITCompiler.h:

(JSC::DFG::InRecord::InRecord):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileIn):

  • jit/JITInlineCacheGenerator.cpp:

(JSC::JITByIdGenerator::finalize):

  • jit/Repatch.cpp:

(JSC::replaceWithJump):
(JSC::patchJumpToGetByIdStub):
(JSC::tryCachePutByID):
(JSC::tryBuildPutByIdList):
(JSC::tryRepatchIn):
(JSC::resetGetByID):
(JSC::resetPutByID):
(JSC::resetIn):

Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r160457 r160493  
     12013-12-11  Filip Pizlo  <fpizlo@apple.com>
     2
     3        ARM64: Hang running pdfjs test, suspect DFG generated code for "in"
     4        https://bugs.webkit.org/show_bug.cgi?id=124727
     5        <rdar://problem/15566923>
     6
     7        Reviewed by Michael Saboff.
     8       
     9        Get rid of In's hackish use of StructureStubInfo. Previously it was using hotPathBegin,
     10        and it was the only IC that used that field, which was wasteful. Moreover, it used it
     11        to store two separate locations: the label for patching the jump and the label right
     12        after the jump. The code was relying on those two being the same label, which is true
     13        on X86 and some other platforms, but it isn't true on ARM64.
     14       
     15        This gets rid of hotPathBegin and makes In express those two locations as offsets from
     16        the callReturnLocation, which is analogous to what the other IC's do.
     17       
     18        This fixes a bug where any successful In patching would result in a trivially infinite
     19        loop - and hence a hang - on ARM64.
     20
     21        * bytecode/StructureStubInfo.h:
     22        * dfg/DFGJITCompiler.cpp:
     23        (JSC::DFG::JITCompiler::link):
     24        * dfg/DFGJITCompiler.h:
     25        (JSC::DFG::InRecord::InRecord):
     26        * dfg/DFGSpeculativeJIT.cpp:
     27        (JSC::DFG::SpeculativeJIT::compileIn):
     28        * jit/JITInlineCacheGenerator.cpp:
     29        (JSC::JITByIdGenerator::finalize):
     30        * jit/Repatch.cpp:
     31        (JSC::replaceWithJump):
     32        (JSC::patchJumpToGetByIdStub):
     33        (JSC::tryCachePutByID):
     34        (JSC::tryBuildPutByIdList):
     35        (JSC::tryRepatchIn):
     36        (JSC::resetGetByID):
     37        (JSC::resetPutByID):
     38        (JSC::resetIn):
     39
    1402013-12-11  Joseph Pecoraro  <pecoraro@apple.com>
    241
  • trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h

    r158820 r160493  
    235235        int32_t deltaCallToDone;
    236236        int32_t deltaCallToStorageLoad;
    237         int32_t deltaCallToStructCheck;
     237        int32_t deltaCallToJump;
    238238        int32_t deltaCallToSlowCase;
    239239        int32_t deltaCheckImmToCall;
     
    292292    RefPtr<JITStubRoutine> stubRoutine;
    293293    CodeLocationCall callReturnLocation;
    294     CodeLocationLabel hotPathBegin; // FIXME: This is only used by DFG In IC.
    295294    RefPtr<WatchpointsOnStructureStubInfo> watchpoints;
    296295};
  • trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp

    r159886 r160493  
    222222    for (unsigned i = 0; i < m_ins.size(); ++i) {
    223223        StructureStubInfo& info = *m_ins[i].m_stubInfo;
    224         CodeLocationLabel jump = linkBuffer.locationOf(m_ins[i].m_jump);
    225224        CodeLocationCall callReturnLocation = linkBuffer.locationOf(m_ins[i].m_slowPathGenerator->call());
    226         info.hotPathBegin = jump;
     225        info.patch.deltaCallToDone = differenceBetweenCodePtr(callReturnLocation, linkBuffer.locationOf(m_ins[i].m_done));
     226        info.patch.deltaCallToJump = differenceBetweenCodePtr(callReturnLocation, linkBuffer.locationOf(m_ins[i].m_jump));
    227227        info.callReturnLocation = callReturnLocation;
    228228        info.patch.deltaCallToSlowCase = differenceBetweenCodePtr(callReturnLocation, linkBuffer.locationOf(m_ins[i].m_slowPathGenerator->label()));
  • trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.h

    r159826 r160493  
    8282struct InRecord {
    8383    InRecord(
    84         MacroAssembler::PatchableJump jump, SlowPathGenerator* slowPathGenerator,
    85         StructureStubInfo* stubInfo)
     84        MacroAssembler::PatchableJump jump, MacroAssembler::Label done,
     85        SlowPathGenerator* slowPathGenerator, StructureStubInfo* stubInfo)
    8686        : m_jump(jump)
     87        , m_done(done)
    8788        , m_slowPathGenerator(slowPathGenerator)
    8889        , m_stubInfo(stubInfo)
     
    9192   
    9293    MacroAssembler::PatchableJump m_jump;
     94    MacroAssembler::Label m_done;
    9395    SlowPathGenerator* m_slowPathGenerator;
    9496    StructureStubInfo* m_stubInfo;
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r160411 r160493  
    973973
    974974            use(node->child1());
    975                
     975           
    976976            MacroAssembler::PatchableJump jump = m_jit.patchableJump();
     977            MacroAssembler::Label done = m_jit.label();
    977978           
    978979            OwnPtr<SlowPathGenerator> slowPath = slowPathCall(
     
    987988            stubInfo->patch.registersFlushed = false;
    988989           
    989             m_jit.addIn(InRecord(jump, slowPath.get(), stubInfo));
     990            m_jit.addIn(InRecord(jump, done, slowPath.get(), stubInfo));
    990991            addSlowPathGenerator(slowPath.release());
    991992               
  • trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp

    r158820 r160493  
    7777    m_stubInfo->patch.deltaCheckImmToCall = MacroAssembler::differenceBetweenCodePtr(
    7878        fastPath.locationOf(m_structureImm), callReturnLocation);
    79     m_stubInfo->patch.deltaCallToStructCheck = MacroAssembler::differenceBetweenCodePtr(
     79    m_stubInfo->patch.deltaCallToJump = MacroAssembler::differenceBetweenCodePtr(
    8080        callReturnLocation, fastPath.locationOf(m_structureCheck));
    8181#if USE(JSVALUE64)
  • trunk/Source/JavaScriptCore/jit/Repatch.cpp

    r159593 r160493  
    175175    repatchBuffer.relink(
    176176        stubInfo.callReturnLocation.jumpAtOffset(
    177             stubInfo.patch.deltaCallToStructCheck),
     177            stubInfo.patch.deltaCallToJump),
    178178        CodeLocationLabel(target));
    179179}
     
    458458        repatchBuffer.relink(
    459459            stubInfo.callReturnLocation.jumpAtOffset(
    460                 stubInfo.patch.deltaCallToStructCheck),
     460                stubInfo.patch.deltaCallToJump),
    461461            CodeLocationLabel(stubRoutine->code().code()));
    462462        return;
     
    10341034            repatchBuffer.relink(
    10351035                stubInfo.callReturnLocation.jumpAtOffset(
    1036                     stubInfo.patch.deltaCallToStructCheck),
     1036                    stubInfo.patch.deltaCallToJump),
    10371037                CodeLocationLabel(stubInfo.stubRoutine->code().code()));
    10381038            repatchCall(repatchBuffer, stubInfo.callReturnLocation, appropriateListBuildingPutByIdFunction(slot, putKind));
     
    11381138       
    11391139        RepatchBuffer repatchBuffer(codeBlock);
    1140         repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToStructCheck), CodeLocationLabel(stubRoutine->code().code()));
     1140        repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), CodeLocationLabel(stubRoutine->code().code()));
    11411141       
    11421142        if (list->isFull())
     
    11821182    int listIndex;
    11831183   
    1184     CodeLocationLabel successLabel = stubInfo.hotPathBegin;
     1184    CodeLocationLabel successLabel = stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToDone);
    11851185    CodeLocationLabel slowCaseLabel;
    11861186   
     
    12601260   
    12611261    RepatchBuffer repatchBuffer(codeBlock);
    1262     repatchBuffer.relink(stubInfo.hotPathBegin.jumpAtOffset(0), CodeLocationLabel(stubRoutine->code().code()));
     1262    repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), CodeLocationLabel(stubRoutine->code().code()));
    12631263   
    12641264    return listIndex < (POLYMORPHIC_LIST_CACHE_SIZE - 1);
     
    14331433    repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabelCompactAtOffset(stubInfo.patch.deltaCallToPayloadLoadOrStore), 0);
    14341434#endif
    1435     repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToStructCheck), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
     1435    repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
    14361436}
    14371437
     
    14671467    repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabel32AtOffset(stubInfo.patch.deltaCallToPayloadLoadOrStore), 0);
    14681468#endif
    1469     repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToStructCheck), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
     1469    repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
    14701470}
    14711471
    14721472void resetIn(RepatchBuffer& repatchBuffer, StructureStubInfo& stubInfo)
    14731473{
    1474     repatchBuffer.relink(stubInfo.hotPathBegin.jumpAtOffset(0), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
     1474    repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
    14751475}
    14761476
Note: See TracChangeset for help on using the changeset viewer.