Changeset 100095 in webkit


Ignore:
Timestamp:
Nov 13, 2011 2:44:16 PM (12 years ago)
Author:
yuqiang.xian@intel.com
Message:

Fix silent spilling/filling GPRs in DFG 32_64
​https://bugs.webkit.org/show_bug.cgi?id=72201

Reviewed by Gavin Barraclough.

Current silentSpillGPR/silentFillGPR may not work as expected for some
cases in 32_64. If there's a JSValue which was retained by two GPRs,
we may end up failing to spill/fill some GPRs or redundantly
spilling/filling some GPRs. For example, if we tend to exclude "eax"
from spilling while a JSValue is retained by both "eax" and "edx",
then "edx" won't be spilled as well (wrong). And if another JSValue is
retained by "ecx" and "ebx", both "ecx" and "ebx" will be spilled
twice. The similar problem applies to silentFillGPR.
The fix is to make silentSpillGPR/silentFillGPR more straightforward,
i.e., spilling/filling based on the GPR instead of the virtual
register. FPR spilling/filling is also modified accordingly to make it
consistent with GPR spilling/filling.

  • dfg/DFGJITCodeGenerator.h:

(JSC::DFG::JITCodeGenerator::silentSpillGPR):
(JSC::DFG::JITCodeGenerator::silentSpillFPR):
(JSC::DFG::JITCodeGenerator::silentFillGPR):
(JSC::DFG::JITCodeGenerator::silentFillFPR):
(JSC::DFG::JITCodeGenerator::silentSpillAllRegisters):
(JSC::DFG::JITCodeGenerator::silentFillAllRegisters):

Location:
trunk/Source/JavaScriptCore
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r100086 r100095  
     12011-11-13  Yuqiang Xian  <yuqiang.xian@intel.com>
     2
     3        Fix silent spilling/filling GPRs in DFG 32_64
     4        https://bugs.webkit.org/show_bug.cgi?id=72201
     5
     6        Reviewed by Gavin Barraclough.
     7
     8        Current silentSpillGPR/silentFillGPR may not work as expected for some
     9        cases in 32_64. If there's a JSValue which was retained by two GPRs,
     10        we may end up failing to spill/fill some GPRs or redundantly
     11        spilling/filling some GPRs. For example, if we tend to exclude "eax"
     12        from spilling while a JSValue is retained by both "eax" and "edx",
     13        then "edx" won't be spilled as well (wrong). And if another JSValue is
     14        retained by "ecx" and "ebx", both "ecx" and "ebx" will be spilled
     15        twice. The similar problem applies to silentFillGPR.
     16        The fix is to make silentSpillGPR/silentFillGPR more straightforward,
     17        i.e., spilling/filling based on the GPR instead of the virtual
     18        register. FPR spilling/filling is also modified accordingly to make it
     19        consistent with GPR spilling/filling.
     20
     21        * dfg/DFGJITCodeGenerator.h:
     22        (JSC::DFG::JITCodeGenerator::silentSpillGPR):
     23        (JSC::DFG::JITCodeGenerator::silentSpillFPR):
     24        (JSC::DFG::JITCodeGenerator::silentFillGPR):
     25        (JSC::DFG::JITCodeGenerator::silentFillFPR):
     26        (JSC::DFG::JITCodeGenerator::silentSpillAllRegisters):
     27        (JSC::DFG::JITCodeGenerator::silentFillAllRegisters):
     28
    1292011-11-12  Laszlo Gombos  <laszlo.1.gombos@nokia.com>
    230

  • trunk/Source/JavaScriptCore/dfg/DFGJITCodeGenerator.h

    r99904 r100095  
    263263    void clearGenerationInfo();
    264264
    265 #if USE(JSVALUE32_64)
    266     bool registersMatched(GenerationInfo& info, GPRReg exclude, GPRReg exclude2)
    267     {
    268         ASSERT(info.registerFormat() != DataFormatNone);
    269         ASSERT(info.registerFormat() != DataFormatDouble);
    270         ASSERT(info.registerFormat() != DataFormatJSDouble);
    271 
    272         if (exclude == InvalidGPRReg && exclude2 == InvalidGPRReg)
    273             return false;
    274 
    275         bool matched = false;
    276         if (exclude2 == InvalidGPRReg)
    277             matched = (!(info.registerFormat() & DataFormatJS)) ? (info.gpr() == exclude) :  (info.tagGPR() == exclude || info.payloadGPR() == exclude);
    278         else if (exclude == InvalidGPRReg)
    279             matched = (!(info.registerFormat() & DataFormatJS)) ? (info.gpr() == exclude2) :  (info.tagGPR() == exclude2 || info.payloadGPR() == exclude2);
    280         else
    281             matched = (!(info.registerFormat() & DataFormatJS)) ? (info.gpr() == exclude || info.gpr() == exclude2) :  (info.tagGPR() == exclude || info.tagGPR() == exclude2 || info.payloadGPR() == exclude || info.payloadGPR() == exclude2);
    282         return matched;
    283     }
    284 #endif
    285 
    286265    // These methods are used when generating 'unexpected'
    287266    // calls out from JIT code to C++ helper routines -
    … …  
    289268    // slots in the RegisterFile without changing any state
    290269    // in the GenerationInfo.
    291     void silentSpillGPR(VirtualRegister spillMe, GPRReg exclude = InvalidGPRReg, GPRReg exclude2 = InvalidGPRReg)
     270    void silentSpillGPR(VirtualRegister spillMe, GPRReg source)
    292271    {
    293272        GenerationInfo& info = m_generationInfo[spillMe];
    … …  
    295274        ASSERT(info.registerFormat() != DataFormatDouble);
    296275
     276        if (!info.needsSpill())
     277            return;
     278
     279        DataFormat registerFormat = info.registerFormat();
     280
    297281#if USE(JSVALUE64)
    298         UNUSED_PARAM(exclude2);
    299         if (!info.needsSpill() || (info.gpr() == exclude))
    300 #elif USE(JSVALUE32_64)
    301         if (!info.needsSpill() || registersMatched(info, exclude, exclude2))
    302 #endif
    303             return;
    304 
    305         DataFormat registerFormat = info.registerFormat();
    306 
    307 #if USE(JSVALUE64)
     282        ASSERT(info.gpr() == source);
    308283        if (registerFormat == DataFormatInteger)
    309             m_jit.store32(info.gpr(), JITCompiler::addressFor(spillMe));
     284            m_jit.store32(source, JITCompiler::addressFor(spillMe));
    310285        else {
    311286            ASSERT(registerFormat & DataFormatJS || registerFormat == DataFormatCell || registerFormat == DataFormatStorage);
    312             m_jit.storePtr(info.gpr(), JITCompiler::addressFor(spillMe));
     287            m_jit.storePtr(source, JITCompiler::addressFor(spillMe));
    313288        }
    314289#elif USE(JSVALUE32_64)
    315         if (registerFormat == DataFormatInteger || registerFormat == DataFormatBoolean)
    316             m_jit.store32(info.gpr(), JITCompiler::payloadFor(spillMe));
    317         else if (registerFormat == DataFormatCell)
    318             m_jit.storePtr(info.gpr(), JITCompiler::payloadFor(spillMe));
    319         else if (registerFormat == DataFormatStorage)
    320             m_jit.storePtr(info.gpr(), JITCompiler::addressFor(spillMe));
    321         else {
    322             ASSERT(registerFormat & DataFormatJS);
    323             m_jit.store32(info.tagGPR(), JITCompiler::tagFor(spillMe));
    324             m_jit.store32(info.payloadGPR(), JITCompiler::payloadFor(spillMe));
    325         }
    326 #endif
    327     }
    328     void silentSpillFPR(VirtualRegister spillMe, FPRReg exclude = InvalidFPRReg)
     290        if (registerFormat & DataFormatJS) {
     291            ASSERT(info.tagGPR() == source || info.payloadGPR() == source);
     292            m_jit.store32(source, source == info.tagGPR() ? JITCompiler::tagFor(spillMe) : JITCompiler::payloadFor(spillMe));
     293        } else {
     294            ASSERT(info.gpr() == source);
     295            m_jit.store32(source, JITCompiler::payloadFor(spillMe));
     296        }
     297#endif
     298    }
     299    void silentSpillFPR(VirtualRegister spillMe, FPRReg source)
    329300    {
    330301        GenerationInfo& info = m_generationInfo[spillMe];
    331302        ASSERT(info.registerFormat() == DataFormatDouble);
    332303
    333         if (info.fpr() == exclude)
    334             return;
    335304        if (!info.needsSpill()) {
    336305            // it's either a constant or it's already been spilled
    … …  
    342311        ASSERT(!at(info.nodeIndex()).hasConstant());
    343312        ASSERT(info.spillFormat() == DataFormatNone);
    344 
    345         m_jit.storeDouble(info.fpr(), JITCompiler::addressFor(spillMe));
    346     }
    347 
    348     void silentFillGPR(VirtualRegister spillMe, GPRReg exclude = InvalidGPRReg, GPRReg exclude2 = InvalidGPRReg)
     313        ASSERT(info.fpr() == source);
     314
     315        m_jit.storeDouble(source, JITCompiler::addressFor(spillMe));
     316    }
     317
     318    void silentFillGPR(VirtualRegister spillMe, GPRReg target)
    349319    {
    350320        GenerationInfo& info = m_generationInfo[spillMe];
    351 #if USE(JSVALUE64)
    352         UNUSED_PARAM(exclude2);
    353         if (info.gpr() == exclude)
    354 #elif USE(JSVALUE32_64)
    355         if (registersMatched(info, exclude, exclude2))
    356 #endif
    357             return;
    358321
    359322        NodeIndex nodeIndex = info.nodeIndex();
    … …  
    364327
    365328        if (registerFormat == DataFormatInteger) {
     329            ASSERT(info.gpr() == target);
    366330            ASSERT(isJSInteger(info.registerFormat()));
    367331            if (node.hasConstant()) {
    368332                ASSERT(isInt32Constant(nodeIndex));
    369                 m_jit.move(Imm32(valueOfInt32Constant(nodeIndex)), info.gpr());
     333                m_jit.move(Imm32(valueOfInt32Constant(nodeIndex)), target);
    370334            } else
    371                 m_jit.load32(JITCompiler::payloadFor(spillMe), info.gpr());
     335                m_jit.load32(JITCompiler::payloadFor(spillMe), target);
    372336            return;
    373337        }
    … …  
    377341            ASSERT_NOT_REACHED();
    378342#elif USE(JSVALUE32_64)
     343            ASSERT(info.gpr() == target);
    379344            if (node.hasConstant()) {
    380345                ASSERT(isBooleanConstant(nodeIndex));
    381                 m_jit.move(Imm32(valueOfBooleanConstant(nodeIndex)), info.gpr());
     346                m_jit.move(Imm32(valueOfBooleanConstant(nodeIndex)), target);
    382347            } else
    383                 m_jit.load32(JITCompiler::payloadFor(spillMe), info.gpr());
     348                m_jit.load32(JITCompiler::payloadFor(spillMe), target);
    384349#endif
    385350            return;
    … …  
    387352
    388353        if (registerFormat == DataFormatCell) {
     354            ASSERT(info.gpr() == target);
    389355            if (node.isConstant()) {
    390356                JSValue value = valueOfJSConstant(nodeIndex);
    391357                ASSERT(value.isCell());
    392                 m_jit.move(ImmPtr(value.asCell()), info.gpr());
     358                m_jit.move(ImmPtr(value.asCell()), target);
    393359            } else
    394                 m_jit.loadPtr(JITCompiler::payloadFor(spillMe), info.gpr());
     360                m_jit.loadPtr(JITCompiler::payloadFor(spillMe), target);
    395361            return;
    396362        }
    397363
    398364        if (registerFormat == DataFormatStorage) {
    399             m_jit.loadPtr(JITCompiler::addressFor(spillMe), info.gpr());
     365            ASSERT(info.gpr() == target);
     366            m_jit.loadPtr(JITCompiler::addressFor(spillMe), target);
    400367            return;
    401368        }
    … …  
    403370        ASSERT(registerFormat & DataFormatJS);
    404371#if USE(JSVALUE64)
     372        ASSERT(info.gpr() == target);
    405373        if (node.hasConstant())
    406             m_jit.move(valueOfJSConstantAsImmPtr(nodeIndex), info.gpr());
     374            m_jit.move(valueOfJSConstantAsImmPtr(nodeIndex), target);
    407375        else if (info.spillFormat() == DataFormatInteger) {
    408376            ASSERT(registerFormat == DataFormatJSInteger);
    409             m_jit.load32(JITCompiler::payloadFor(spillMe), info.gpr());
    410             m_jit.orPtr(GPRInfo::tagTypeNumberRegister, info.gpr());
     377            m_jit.load32(JITCompiler::payloadFor(spillMe), target);
     378            m_jit.orPtr(GPRInfo::tagTypeNumberRegister, target);
    411379        } else if (info.spillFormat() == DataFormatDouble) {
    412380            ASSERT(registerFormat == DataFormatJSDouble);
    413             m_jit.loadPtr(JITCompiler::addressFor(spillMe), info.gpr());
    414             m_jit.subPtr(GPRInfo::tagTypeNumberRegister, info.gpr());
     381            m_jit.loadPtr(JITCompiler::addressFor(spillMe), target);
     382            m_jit.subPtr(GPRInfo::tagTypeNumberRegister, target);
    415383        } else
    416             m_jit.loadPtr(JITCompiler::addressFor(spillMe), info.gpr());
     384            m_jit.loadPtr(JITCompiler::addressFor(spillMe), target);
    417385#else
    418         if (node.hasConstant())
    419             m_jit.emitLoad(valueOfJSConstant(nodeIndex), info.tagGPR(), info.payloadGPR());
    420         else if (info.spillFormat() == DataFormatInteger) {
     386        ASSERT(info.tagGPR() == target || info.payloadGPR() == target);
     387        if (node.hasConstant()) {
     388            JSValue v = valueOfJSConstant(nodeIndex);
     389            m_jit.move(info.tagGPR() == target ? Imm32(v.tag()) : Imm32(v.payload()), target);
     390        } else if (info.spillFormat() == DataFormatInteger) {
    421391            ASSERT(registerFormat == DataFormatJSInteger);
    422             m_jit.load32(JITCompiler::payloadFor(spillMe), info.payloadGPR());
    423             m_jit.move(TrustedImm32(JSValue::Int32Tag), info.tagGPR());
     392            if (info.payloadGPR() == target)
     393                m_jit.load32(JITCompiler::payloadFor(spillMe), target);
     394            else
     395                m_jit.move(TrustedImm32(JSValue::Int32Tag), target);
    424396        } else
    425             m_jit.emitLoad(nodeIndex, info.tagGPR(), info.payloadGPR());
    426 #endif
    427     }
    428 
    429     void silentFillFPR(VirtualRegister spillMe, GPRReg canTrample, FPRReg exclude = InvalidFPRReg)
     397            m_jit.load32(info.tagGPR() == target ? JITCompiler::tagFor(spillMe) : JITCompiler::payloadFor(spillMe), target);
     398#endif
     399    }
     400
     401    void silentFillFPR(VirtualRegister spillMe, GPRReg canTrample, FPRReg target)
    430402    {
    431403        GenerationInfo& info = m_generationInfo[spillMe];
    432         if (info.fpr() == exclude)
    433             return;
     404        ASSERT(info.fpr() == target);
    434405
    435406        NodeIndex nodeIndex = info.nodeIndex();
    … …  
    441412            ASSERT(isNumberConstant(nodeIndex));
    442413            m_jit.move(ImmPtr(bitwise_cast<void*>(valueOfNumberConstant(nodeIndex))), canTrample);
    443             m_jit.movePtrToDouble(canTrample, info.fpr());
     414            m_jit.movePtrToDouble(canTrample, target);
    444415            return;
    445416        }
    … …  
    449420            ASSERT(info.spillFormat() & DataFormatJS);
    450421            m_jit.loadPtr(JITCompiler::addressFor(spillMe), canTrample);
    451             unboxDouble(canTrample, info.fpr());
    452             return;
    453         }
    454 
    455         m_jit.loadDouble(JITCompiler::addressFor(spillMe), info.fpr());
     422            unboxDouble(canTrample, target);
     423            return;
     424        }
     425
     426        m_jit.loadDouble(JITCompiler::addressFor(spillMe), target);
    456427#elif USE(JSVALUE32_64)
    457428        UNUSED_PARAM(canTrample);
    458429        ASSERT(info.registerFormat() == DataFormatDouble || info.registerFormat() == DataFormatJSDouble);
    459         m_jit.emitLoadDouble(nodeIndex, info.fpr());
     430        m_jit.emitLoadDouble(nodeIndex, target);
    460431#endif
    461432    }
    462433
    463434    void silentSpillAllRegisters(GPRReg exclude, GPRReg exclude2 = InvalidGPRReg)
     435    {
     436        for (gpr_iterator iter = m_gprs.begin(); iter != m_gprs.end(); ++iter) {
     437            GPRReg gpr = iter.regID();
     438            if (iter.name() != InvalidVirtualRegister && gpr != exclude && gpr != exclude2)
     439                silentSpillGPR(iter.name(), gpr);
     440        }
     441        for (fpr_iterator iter = m_fprs.begin(); iter != m_fprs.end(); ++iter) {
     442            if (iter.name() != InvalidVirtualRegister)
     443                silentSpillFPR(iter.name(), iter.regID());
     444        }
     445    }
     446    void silentSpillAllRegisters(FPRReg exclude)
    464447    {
    465448        for (gpr_iterator iter = m_gprs.begin(); iter != m_gprs.end(); ++iter) {
    466449            if (iter.name() != InvalidVirtualRegister)
    467                 silentSpillGPR(iter.name(), exclude, exclude2);
     450                silentSpillGPR(iter.name(), iter.regID());
    468451        }
    469452        for (fpr_iterator iter = m_fprs.begin(); iter != m_fprs.end(); ++iter) {
    470             if (iter.name() != InvalidVirtualRegister)
    471                 silentSpillFPR(iter.name());
    472         }
    473     }
    474     void silentSpillAllRegisters(FPRReg exclude)
    475     {
    476         for (gpr_iterator iter = m_gprs.begin(); iter != m_gprs.end(); ++iter) {
    477             if (iter.name() != InvalidVirtualRegister)
    478                 silentSpillGPR(iter.name());
    479         }
    480         for (fpr_iterator iter = m_fprs.begin(); iter != m_fprs.end(); ++iter) {
    481             if (iter.name() != InvalidVirtualRegister)
    482                 silentSpillFPR(iter.name(), exclude);
     453            FPRReg fpr = iter.regID();
     454            if (iter.name() != InvalidVirtualRegister && fpr != exclude)
     455                silentSpillFPR(iter.name(), fpr);
    483456        }
    484457    }
    … …  
    492465        for (fpr_iterator iter = m_fprs.begin(); iter != m_fprs.end(); ++iter) {
    493466            if (iter.name() != InvalidVirtualRegister)
    494                 silentFillFPR(iter.name(), canTrample);
     467                silentFillFPR(iter.name(), canTrample, iter.regID());
    495468        }
    496469        for (gpr_iterator iter = m_gprs.begin(); iter != m_gprs.end(); ++iter) {
    497             if (iter.name() != InvalidVirtualRegister)
    498                 silentFillGPR(iter.name(), exclude, exclude2);
     470            GPRReg gpr = iter.regID();
     471            if (iter.name() != InvalidVirtualRegister && gpr != exclude && gpr != exclude2)
     472                silentFillGPR(iter.name(), gpr);
    499473        }
    500474    }
    … …  
    504478       
    505479        for (fpr_iterator iter = m_fprs.begin(); iter != m_fprs.end(); ++iter) {
    506             if (iter.name() != InvalidVirtualRegister) {
    507                 ASSERT_UNUSED(exclude, iter.regID() != exclude);
    508                 silentFillFPR(iter.name(), canTrample, exclude);
    509             }
     480            FPRReg fpr = iter.regID();
     481            if (iter.name() != InvalidVirtualRegister && fpr != exclude)
     482                silentFillFPR(iter.name(), canTrample, fpr);
    510483        }
    511484        for (gpr_iterator iter = m_gprs.begin(); iter != m_gprs.end(); ++iter) {
    512485            if (iter.name() != InvalidVirtualRegister)
    513                 silentFillGPR(iter.name());
     486                silentFillGPR(iter.name(), iter.regID());
    514487        }
    515488    }
Note: See TracChangeset for help on using the changeset viewer.