Changeset 284923 in webkit


Ignore:
Timestamp:
Oct 27, 2021, 8:34:42 AM (4 years ago)
Author:
[email protected]
Message:

[JSC][32bit] Fix CSR restore on DFG tail calls, add extra register on ARMv7
​https://bugs.webkit.org/show_bug.cgi?id=230622

Patch by Geza Lore <Geza Lore> on 2021-10-27
Reviewed by Keith Miller.

This re-introduces the patch reverted by
​https://trac.webkit.org/changeset/284911/webkit
with the C_LOOP interpreter now fixed.

The only difference between the original patch and this version is in
LowLevelInterpreter32_64.asm and LowLevelInterpreter64.asm, which
need the PC base (PB) register restored on C_LOOP on return from a
call, as C_LOOP does not seem to handle this as a proper callee save
register (CSR). On non C_LOOP builds, the CSR restore mechanism takes
care of this, so removed the superfluous instructions.

--- Original ChangeLog ---

This patch does two things:

  1. Adds an extra callee save register (CSR) to be available to DFG on

ARMv7. To do this properly required the following:

  1. Implements the necessary shuffling in CallFrameShuffler on 32-bit

architectures that is required to restore CSRs properly after a tail
call on these architectures. This also fixes the remaining failures in
the 32-bit build of the unlinked baseline JIT.

  • bytecode/ValueRecovery.cpp:

(JSC::ValueRecovery::dumpInContext const):

  • bytecode/ValueRecovery.h:

(JSC::ValueRecovery::calleeSaveRegDisplacedInJSStack):
(JSC::ValueRecovery::isInJSStack const):
(JSC::ValueRecovery::dataFormat const):
(JSC::ValueRecovery::withLocalsOffset const):

  • dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::emitCall):

  • jit/CachedRecovery.cpp:

(JSC::CachedRecovery::loadsIntoGPR const):

  • jit/CallFrameShuffleData.cpp:

(JSC::CallFrameShuffleData::setupCalleeSaveRegisters):

  • jit/CallFrameShuffleData.h:
  • jit/CallFrameShuffler.cpp:

(JSC::CallFrameShuffler::CallFrameShuffler):

  • jit/CallFrameShuffler.h:

(JSC::CallFrameShuffler::snapshot const):
(JSC::CallFrameShuffler::addNew):

  • jit/CallFrameShuffler32_64.cpp:

(JSC::CallFrameShuffler::emitLoad):
(JSC::CallFrameShuffler::emitDisplace):

  • jit/GPRInfo.h:

(JSC::GPRInfo::toRegister):
(JSC::GPRInfo::toIndex):

  • jit/RegisterSet.cpp:

(JSC::RegisterSet::dfgCalleeSaveRegisters):

  • llint/LowLevelInterpreter32_64.asm:
  • llint/LowLevelInterpreter64.asm:
Location:
trunk/Source/JavaScriptCore
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r284911 r284923  
     12021-10-27  Geza Lore  <[email protected]>
     2
     3        [JSC][32bit] Fix CSR restore on DFG tail calls, add extra register on ARMv7
     4        https://bugs.webkit.org/show_bug.cgi?id=230622
     5
     6        Reviewed by Keith Miller.
     7
     8        This re-introduces the patch reverted by
     9        https://trac.webkit.org/changeset/284911/webkit
     10        with the C_LOOP interpreter now fixed.
     11
     12        The only difference between the original patch and this version is in
     13        LowLevelInterpreter32_64.asm and LowLevelInterpreter64.asm, which
     14        need the PC base (PB) register restored on C_LOOP on return from a
     15        call, as C_LOOP does not seem to handle this as a proper callee save
     16        register (CSR). On non C_LOOP builds, the CSR restore mechanism takes
     17        care of this, so removed the superfluous instructions.
     18
     19        --- Original ChangeLog ---
     20
     21        This patch does two things:
     22
     23        1. Adds an extra callee save register (CSR) to be available to DFG on
     24        ARMv7. To do this properly required the following:
     25
     26        2. Implements the necessary shuffling in CallFrameShuffler on 32-bit
     27        architectures that is required to restore CSRs properly after a tail
     28        call on these architectures. This also fixes the remaining failures in
     29        the 32-bit build of the unlinked baseline JIT.
     30
     31        * bytecode/ValueRecovery.cpp:
     32        (JSC::ValueRecovery::dumpInContext const):
     33        * bytecode/ValueRecovery.h:
     34        (JSC::ValueRecovery::calleeSaveRegDisplacedInJSStack):
     35        (JSC::ValueRecovery::isInJSStack const):
     36        (JSC::ValueRecovery::dataFormat const):
     37        (JSC::ValueRecovery::withLocalsOffset const):
     38        * dfg/DFGSpeculativeJIT32_64.cpp:
     39        (JSC::DFG::SpeculativeJIT::emitCall):
     40        * jit/CachedRecovery.cpp:
     41        (JSC::CachedRecovery::loadsIntoGPR const):
     42        * jit/CallFrameShuffleData.cpp:
     43        (JSC::CallFrameShuffleData::setupCalleeSaveRegisters):
     44        * jit/CallFrameShuffleData.h:
     45        * jit/CallFrameShuffler.cpp:
     46        (JSC::CallFrameShuffler::CallFrameShuffler):
     47        * jit/CallFrameShuffler.h:
     48        (JSC::CallFrameShuffler::snapshot const):
     49        (JSC::CallFrameShuffler::addNew):
     50        * jit/CallFrameShuffler32_64.cpp:
     51        (JSC::CallFrameShuffler::emitLoad):
     52        (JSC::CallFrameShuffler::emitDisplace):
     53        * jit/GPRInfo.h:
     54        (JSC::GPRInfo::toRegister):
     55        (JSC::GPRInfo::toIndex):
     56        * jit/RegisterSet.cpp:
     57        (JSC::RegisterSet::dfgCalleeSaveRegisters):
     58        * llint/LowLevelInterpreter32_64.asm:
     59        * llint/LowLevelInterpreter64.asm:
     60
    1612021-10-26  Commit Queue  <[email protected]>
    262

  • trunk/Source/JavaScriptCore/bytecode/ValueRecovery.cpp

    r284911 r284923  
    100100        out.print("*int32(", virtualRegister(), ")");
    101101        return;
     102#if USE(JSVALUE32_64)
     103    case Int32TagDisplacedInJSStack:
     104        out.print("*int32Tag(", virtualRegister(), ")");
     105        return;
     106#endif
    102107    case Int52DisplacedInJSStack:
    103108        out.print("*int52(", virtualRegister(), ")");

  • trunk/Source/JavaScriptCore/bytecode/ValueRecovery.h

    r284911 r284923  
    6161    // It's in the stack, at a different location, and it's unboxed.
    6262    Int32DisplacedInJSStack,
     63#if USE(JSVALUE32_64)
     64    Int32TagDisplacedInJSStack, // int32 stored in tag field
     65#endif
    6366    Int52DisplacedInJSStack,
    6467    StrictInt52DisplacedInJSStack,
    … …  
    188191        return result;
    189192    }
    190    
     193
     194#if USE(JSVALUE32_64)
     195    static ValueRecovery calleeSaveRegDisplacedInJSStack(VirtualRegister virtualReg, bool inTag)
     196    {
     197        ValueRecovery result;
     198        UnionType u;
     199        u.virtualReg = virtualReg.offset();
     200        result.m_source = WTFMove(u);
     201        result.m_technique = inTag ? Int32TagDisplacedInJSStack : Int32DisplacedInJSStack;
     202        return result;
     203    }
     204#endif
     205
    191206    static ValueRecovery constant(JSValue value)
    192207    {
    … …  
    259274        case DisplacedInJSStack:
    260275        case Int32DisplacedInJSStack:
     276#if USE(JSVALUE32_64)
     277        case Int32TagDisplacedInJSStack:
     278#endif
    261279        case Int52DisplacedInJSStack:
    262280        case StrictInt52DisplacedInJSStack:
    … …  
    283301        case UnboxedInt32InGPR:
    284302        case Int32DisplacedInJSStack:
     303#if USE(JSVALUE32_64)
     304        case Int32TagDisplacedInJSStack:
     305#endif
    285306            return DataFormatInt32;
    286307        case UnboxedInt52InGPR:
    … …  
    359380        case DisplacedInJSStack:
    360381        case Int32DisplacedInJSStack:
     382#if USE(JSVALUE32_64)
     383        case Int32TagDisplacedInJSStack:
     384#endif
    361385        case DoubleDisplacedInJSStack:
    362386        case CellDisplacedInJSStack:

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp

    r284911 r284923  
    743743            for (unsigned i = numPassedArgs; i < numAllocatedArgs; ++i)
    744744                shuffleData.args[i] = ValueRecovery::constant(jsUndefined());
     745
     746            shuffleData.setupCalleeSaveRegisters(m_jit.codeBlock());
    745747        } else {
    746748            m_jit.store32(MacroAssembler::TrustedImm32(numPassedArgs), m_jit.calleeFramePayloadSlot(CallFrameSlot::argumentCountIncludingThis));

  • trunk/Source/JavaScriptCore/jit/CachedRecovery.cpp

    r284911 r284923  
    5353    switch (recovery().technique()) {
    5454    case Int32DisplacedInJSStack:
    55 #if USE(JSVALUE64)
     55#if USE(JSVALUE32_64)
     56    case Int32TagDisplacedInJSStack:
     57#elif USE(JSVALUE64)
    5658    case Int52DisplacedInJSStack:
    5759    case StrictInt52DisplacedInJSStack:

  • trunk/Source/JavaScriptCore/jit/CallFrameShuffleData.cpp

    r284911 r284923  
    3434namespace JSC {
    3535
    36 #if USE(JSVALUE64)
    37 
    3836void CallFrameShuffleData::setupCalleeSaveRegisters(CodeBlock* codeBlock)
    3937{
    … …  
    5048            continue;
    5149
    52         VirtualRegister saveSlot { entry.offsetAsIndex() };
     50        int saveSlotIndexInCPURegisters = entry.offsetAsIndex();
     51
     52#if USE(JSVALUE64)
     53        // CPU registers are the same size as virtual registers
     54        VirtualRegister saveSlot { saveSlotIndexInCPURegisters };
    5355        registers[entry.reg()]
    5456            = ValueRecovery::displacedInJSStack(saveSlot, DataFormatJS);
     57#elif USE(JSVALUE32_64)
     58        // On 32-bit architectures, 2 callee saved registers may be packed into the same slot
     59        static_assert(!PayloadOffset || !TagOffset);
     60        static_assert(PayloadOffset == 4 || TagOffset == 4);
     61        bool inTag = (saveSlotIndexInCPURegisters & 1) == !!TagOffset;
     62        if (saveSlotIndexInCPURegisters < 0)
     63            saveSlotIndexInCPURegisters -= 1; // Round towards -inf
     64        VirtualRegister saveSlot { saveSlotIndexInCPURegisters / 2 };
     65        registers[entry.reg()]
     66            = ValueRecovery::calleeSaveRegDisplacedInJSStack(saveSlot, inTag);
     67#endif
    5568    }
    5669
    … …  
    6275            continue;
    6376
     77#if USE(JSVALUE64)
    6478        registers[reg] = ValueRecovery::inRegister(reg, DataFormatJS);
     79#elif USE(JSVALUE32_64)
     80        registers[reg] = ValueRecovery::inRegister(reg, DataFormatInt32);
     81#endif
    6582    }
    6683}
    67 
    68 #endif // USE(JSVALUE64)
    6984
    7085} // namespace JSC

  • trunk/Source/JavaScriptCore/jit/CallFrameShuffleData.h

    r284911 r284923  
    4545    unsigned numPassedArgs { UINT_MAX };
    4646    unsigned numParameters { UINT_MAX }; // On our machine frame.
     47    RegisterMap<ValueRecovery> registers;
    4748#if USE(JSVALUE64)
    48     RegisterMap<ValueRecovery> registers;
    4949    GPRReg numberTagRegister { InvalidGPRReg };
     50#endif
    5051
    5152    void setupCalleeSaveRegisters(CodeBlock*);
    5253    void setupCalleeSaveRegisters(const RegisterAtOffsetList*);
    53 #endif
    5454    ValueRecovery callee;
    5555};

  • trunk/Source/JavaScriptCore/jit/CallFrameShuffler.cpp

    r284911 r284923  
    5353        m_lockedRegisters.clear(FPRInfo::toRegister(i));
    5454
    55 #if USE(JSVALUE64)
    56     // ... as well as the runtime registers on 64-bit architectures.
    57     // However do not use these registers on 32-bit architectures since
    58     // saving and restoring callee-saved registers in CallFrameShuffler isn't supported
    59     // on 32-bit architectures yet.
     55    // ... as well as the callee saved registers
    6056    m_lockedRegisters.exclude(RegisterSet::vmCalleeSaveRegisters());
    61 #endif
    6257
    6358    ASSERT(!data.callee.isInJSStack() || data.callee.virtualRegister().isLocal());
    … …  
    6964    }
    7065
    71 #if USE(JSVALUE64)
    7266    for (Reg reg = Reg::first(); reg <= Reg::last(); reg = reg.next()) {
    7367        if (!data.registers[reg].isSet())
    7468            continue;
    7569
    76         if (reg.isGPR())
     70        if (reg.isGPR()) {
     71#if USE(JSVALUE64)
    7772            addNew(JSValueRegs(reg.gpr()), data.registers[reg]);
    78         else
     73#elif USE(JSVALUE32_64)
     74            addNew(reg.gpr(), data.registers[reg]);
     75#endif
     76        } else
    7977            addNew(reg.fpr(), data.registers[reg]);
    8078    }
    8179
     80#if USE(JSVALUE64)
    8281    m_numberTagRegister = data.numberTagRegister;
    8382    if (m_numberTagRegister != InvalidGPRReg)

  • trunk/Source/JavaScriptCore/jit/CallFrameShuffler.h

    r284911 r284923  
    118118#if USE(JSVALUE64)
    119119            data.registers[reg] = cachedRecovery->recovery();
     120#elif USE(JSVALUE32_64)
     121            ValueRecovery recovery = cachedRecovery->recovery();
     122            if (recovery.technique() == DisplacedInJSStack) {
     123                JSValueRegs wantedJSValueReg = cachedRecovery->wantedJSValueRegs();
     124                ASSERT(reg == wantedJSValueReg.payloadGPR() || reg == wantedJSValueReg.tagGPR());
     125                bool inTag = reg == wantedJSValueReg.tagGPR();
     126                data.registers[reg] = ValueRecovery::calleeSaveRegDisplacedInJSStack(recovery.virtualRegister(), inTag);
     127            } else
     128                data.registers[reg] = recovery;
    120129#else
    121130            RELEASE_ASSERT_NOT_REACHED();
    … …  
    665674    }
    666675
     676#if USE(JSVALUE32_64)
     677    void addNew(GPRReg gpr, ValueRecovery recovery)
     678    {
     679        ASSERT(gpr != InvalidGPRReg && !m_newRegisters[gpr]);
     680        ASSERT(recovery.technique() == Int32DisplacedInJSStack
     681            || recovery.technique() == Int32TagDisplacedInJSStack);
     682        CachedRecovery* cachedRecovery = addCachedRecovery(recovery);
     683        if (JSValueRegs oldRegs { cachedRecovery->wantedJSValueRegs() }) {
     684            // Combine with the other CSR in the same virtual register slot
     685            ASSERT(oldRegs.tagGPR() == InvalidGPRReg);
     686            ASSERT(oldRegs.payloadGPR() != InvalidGPRReg && oldRegs.payloadGPR() != gpr);
     687            if (recovery.technique() == Int32DisplacedInJSStack) {
     688                ASSERT(cachedRecovery->recovery().technique() == Int32TagDisplacedInJSStack);
     689                cachedRecovery->setWantedJSValueRegs(JSValueRegs(oldRegs.payloadGPR(), gpr));
     690            } else {
     691                ASSERT(cachedRecovery->recovery().technique() == Int32DisplacedInJSStack);
     692                cachedRecovery->setWantedJSValueRegs(JSValueRegs(gpr, oldRegs.payloadGPR()));
     693            }
     694            cachedRecovery->setRecovery(
     695                ValueRecovery::displacedInJSStack(recovery.virtualRegister(), DataFormatJS));
     696        } else
     697            cachedRecovery->setWantedJSValueRegs(JSValueRegs::payloadOnly(gpr));
     698        m_newRegisters[gpr] = cachedRecovery;
     699    }
     700#endif
     701
    667702    void addNew(FPRReg fpr, ValueRecovery recovery)
    668703    {

  • trunk/Source/JavaScriptCore/jit/CallFrameShuffler32_64.cpp

    r284911 r284923  
    125125            resultGPR = getFreeGPR();
    126126        ASSERT(resultGPR != InvalidGPRReg);
    127         m_jit.loadPtr(address.withOffset(PayloadOffset), resultGPR);
    128         updateRecovery(location,
     127        if (location.recovery().technique() == Int32TagDisplacedInJSStack)
     128            m_jit.loadPtr(address.withOffset(TagOffset), resultGPR);
     129        else
     130            m_jit.loadPtr(address.withOffset(PayloadOffset), resultGPR);
     131        updateRecovery(location,
    129132            ValueRecovery::inGPR(resultGPR, location.recovery().dataFormat()));
    130133        if (verbose)
    … …  
    191194        ASSERT(!m_lockedRegisters.get(wantedTagGPR));
    192195        if (CachedRecovery* currentTag { m_registers[wantedTagGPR] }) {
    193             if (currentTag == &location) {
    194                 if (verbose)
    195                     dataLog("   + ", wantedTagGPR, " is OK\n");
    196             } else {
    197                 // This can never happen on 32bit platforms since we
    198                 // have at most one wanted JSValueRegs, for the
    199                 // callee, and no callee-save registers.
    200                 RELEASE_ASSERT_NOT_REACHED();
    201             }
     196            RELEASE_ASSERT(currentTag == &location);
     197            if (verbose)
     198                dataLog("   + ", wantedTagGPR, " is OK\n");
    202199        }
    203200    }
    … …  
    206203        ASSERT(!m_lockedRegisters.get(wantedPayloadGPR));
    207204        if (CachedRecovery* currentPayload { m_registers[wantedPayloadGPR] }) {
    208             if (currentPayload == &location) {
    209                 if (verbose)
    210                     dataLog("   + ", wantedPayloadGPR, " is OK\n");
    211             } else {
    212                 // See above
    213                 RELEASE_ASSERT_NOT_REACHED();
    214             }
     205            RELEASE_ASSERT(currentPayload == &location);
     206            if (verbose)
     207                dataLog("   + ", wantedPayloadGPR, " is OK\n");
    215208        }
    216209    }

  • trunk/Source/JavaScriptCore/jit/GPRInfo.h

    r284911 r284923  
    554554public:
    555555    typedef GPRReg RegisterType;
    556     static constexpr unsigned numberOfRegisters = 9;
     556    static constexpr unsigned numberOfRegisters = 10;
    557557    static constexpr unsigned numberOfArgumentRegisters = NUMBER_OF_ARGUMENT_REGISTERS;
    558558
    … …  
    583583    {
    584584        ASSERT(index < numberOfRegisters);
    585         static const GPRReg registerForIndex[numberOfRegisters] = { regT0, regT1, regT2, regT3, regT4, regT5, regT6, regT7, regCS1 };
     585        static const GPRReg registerForIndex[numberOfRegisters] = { regT0, regT1, regT2, regT3, regT4, regT5, regT6, regT7, regCS0, regCS1 };
    586586        return registerForIndex[index];
    587587    }
    … …  
    599599        ASSERT(static_cast<int>(reg) < 16);
    600600        static const unsigned indexForRegister[16] =
    601             { 0, 1, 2, 3, 7, 6, InvalidIndex, InvalidIndex, 4, 5, 8, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex };
     601            { 0, 1, 2, 3, 7, 6, InvalidIndex, InvalidIndex, 4, 5, 9, 8, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex };
    602602        unsigned result = indexForRegister[reg];
    603603        return result;

  • trunk/Source/JavaScriptCore/jit/RegisterSet.cpp

    r284911 r284923  
    255255#endif
    256256#elif CPU(ARM_THUMB2)
     257    result.set(GPRInfo::regCS0);
    257258    result.set(GPRInfo::regCS1);
    258259#elif CPU(ARM64)

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

    r284911 r284923  
    8080macro dispatchAfterCall(size, opcodeStruct, valueProfileName, dstVirtualRegister, dispatch)
    8181    loadi ArgumentCountIncludingThis + TagOffset[cfr], PC
    82     loadp CodeBlock[cfr], PB
    83     loadp CodeBlock::m_instructionsRawPointer[PB], PB
     82    if C_LOOP or C_LOOP_WIN
     83        # On non C_LOOP builds, CSR restore takes care of this.
     84        loadp CodeBlock[cfr], PB
     85        loadp CodeBlock::m_instructionsRawPointer[PB], PB
     86    end
    8487    get(size, opcodeStruct, dstVirtualRegister, t3)
    8588    storei r1, TagOffset[cfr, t3, 8]

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

    r284330 r284923  
    8383macro dispatchAfterCall(size, opcodeStruct, valueProfileName, dstVirtualRegister, dispatch)
    8484    loadPC()
    85     loadp CodeBlock[cfr], PB
    86     loadp CodeBlock::m_instructionsRawPointer[PB], PB
     85    if C_LOOP or C_LOOP_WIN
     86        # On non C_LOOP builds, CSR restore takes care of this.
     87        loadp CodeBlock[cfr], PB
     88        loadp CodeBlock::m_instructionsRawPointer[PB], PB
     89    end
    8790    get(size, opcodeStruct, dstVirtualRegister, t1)
    8891    storeq r0, [cfr, t1, 8]
Note: See TracChangeset for help on using the changeset viewer.