Changeset 91099 in webkit


Ignore:
Timestamp:
Jul 15, 2011 1:14:27 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

DFG JIT is inconsistent about fusing branches and speculating
integer comparisons for branches.
https://bugs.webkit.org/show_bug.cgi?id=64573

Patch by Filip Pizlo <fpizlo@apple.com> on 2011-07-15
Reviewed by Gavin Barraclough.

This patch moves some of NonSpeculativeJIT's functionality up into the
JITCodeGenerator superclass so that it can be used from both JITs. Now,
in cases where the speculative JIT doesn't want to speculate but still
wants to emit good code, it can reliably emit the same code sequence as
the non-speculative JIT. This patch also extends the non-speculative
JIT's compare optimizations to include compare/branch fusing, and
extends the speculative JIT's compare optimizations to cover StrictEqual.

  • dfg/DFGJITCodeGenerator.cpp:

(JSC::DFG::JITCodeGenerator::isKnownInteger):
(JSC::DFG::JITCodeGenerator::isKnownNumeric):
(JSC::DFG::JITCodeGenerator::nonSpeculativePeepholeBranch):
(JSC::DFG::JITCodeGenerator::nonSpeculativeCompare):

  • dfg/DFGJITCodeGenerator.h:

(JSC::DFG::JITCodeGenerator::detectPeepHoleBranch):

  • dfg/DFGNonSpeculativeJIT.cpp:

(JSC::DFG::NonSpeculativeJIT::compile):

  • dfg/DFGNonSpeculativeJIT.h:
  • dfg/DFGOperations.cpp:
  • dfg/DFGSpeculativeJIT.cpp:

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

  • dfg/DFGSpeculativeJIT.h:
  • wtf/Platform.h:
Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r91095 r91099  
     12011-07-15  Filip Pizlo  <fpizlo@apple.com>
     2
     3        DFG JIT is inconsistent about fusing branches and speculating
     4        integer comparisons for branches.
     5        https://bugs.webkit.org/show_bug.cgi?id=64573
     6
     7        Reviewed by Gavin Barraclough.
     8       
     9        This patch moves some of NonSpeculativeJIT's functionality up into the
     10        JITCodeGenerator superclass so that it can be used from both JITs.  Now,
     11        in cases where the speculative JIT doesn't want to speculate but still
     12        wants to emit good code, it can reliably emit the same code sequence as
     13        the non-speculative JIT.  This patch also extends the non-speculative
     14        JIT's compare optimizations to include compare/branch fusing, and
     15        extends the speculative JIT's compare optimizations to cover StrictEqual.
     16
     17        * dfg/DFGJITCodeGenerator.cpp:
     18        (JSC::DFG::JITCodeGenerator::isKnownInteger):
     19        (JSC::DFG::JITCodeGenerator::isKnownNumeric):
     20        (JSC::DFG::JITCodeGenerator::nonSpeculativePeepholeBranch):
     21        (JSC::DFG::JITCodeGenerator::nonSpeculativeCompare):
     22        * dfg/DFGJITCodeGenerator.h:
     23        (JSC::DFG::JITCodeGenerator::detectPeepHoleBranch):
     24        * dfg/DFGNonSpeculativeJIT.cpp:
     25        (JSC::DFG::NonSpeculativeJIT::compile):
     26        * dfg/DFGNonSpeculativeJIT.h:
     27        * dfg/DFGOperations.cpp:
     28        * dfg/DFGSpeculativeJIT.cpp:
     29        (JSC::DFG::SpeculativeJIT::compare):
     30        (JSC::DFG::SpeculativeJIT::compile):
     31        * dfg/DFGSpeculativeJIT.h:
     32        * wtf/Platform.h:
     33
    1342011-07-14  Gavin Barraclough  <barraclough@apple.com>
    235
  • trunk/Source/JavaScriptCore/dfg/DFGJITCodeGenerator.cpp

    r91041 r91099  
    326326}
    327327
     328bool JITCodeGenerator::isKnownInteger(NodeIndex nodeIndex)
     329{
     330    if (isInt32Constant(nodeIndex))
     331        return true;
     332
     333    GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
     334
     335    DataFormat registerFormat = info.registerFormat();
     336    if (registerFormat != DataFormatNone)
     337        return (registerFormat | DataFormatJS) == DataFormatJSInteger;
     338
     339    DataFormat spillFormat = info.spillFormat();
     340    if (spillFormat != DataFormatNone)
     341        return (spillFormat | DataFormatJS) == DataFormatJSInteger;
     342
     343    ASSERT(isConstant(nodeIndex));
     344    return false;
     345}
     346
     347bool JITCodeGenerator::isKnownNumeric(NodeIndex nodeIndex)
     348{
     349    if (isInt32Constant(nodeIndex) || isDoubleConstant(nodeIndex))
     350        return true;
     351
     352    GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
     353
     354    DataFormat registerFormat = info.registerFormat();
     355    if (registerFormat != DataFormatNone)
     356        return (registerFormat | DataFormatJS) == DataFormatJSInteger
     357            || (registerFormat | DataFormatJS) == DataFormatJSDouble;
     358
     359    DataFormat spillFormat = info.spillFormat();
     360    if (spillFormat != DataFormatNone)
     361        return (spillFormat | DataFormatJS) == DataFormatJSInteger
     362            || (spillFormat | DataFormatJS) == DataFormatJSDouble;
     363
     364    ASSERT(isConstant(nodeIndex));
     365    return false;
     366}
     367
    328368JITCompiler::Call JITCodeGenerator::cachedGetById(GPRReg baseGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget, NodeType nodeType)
    329369{
     
    467507   
    468508    m_jit.addMethodGet(slowCall, structToCompare, protoObj, protoStructToCompare, putFunction);
     509}
     510
     511void JITCodeGenerator::nonSpeculativePeepholeBranch(Node& node, NodeIndex branchNodeIndex, MacroAssembler::RelationalCondition cond, Z_DFGOperation_EJJ helperFunction)
     512{
     513    Node& branchNode = m_jit.graph()[branchNodeIndex];
     514    BlockIndex taken = m_jit.graph().blockIndexForBytecodeOffset(branchNode.takenBytecodeOffset());
     515    BlockIndex notTaken = m_jit.graph().blockIndexForBytecodeOffset(branchNode.notTakenBytecodeOffset());
     516
     517    JITCompiler::ResultCondition callResultCondition = JITCompiler::NonZero;
     518
     519    // The branch instruction will branch to the taken block.
     520    // If taken is next, switch taken with notTaken & invert the branch condition so we can fall through.
     521    if (taken == (m_block + 1)) {
     522        cond = JITCompiler::invert(cond);
     523        callResultCondition = JITCompiler::Zero;
     524        BlockIndex tmp = taken;
     525        taken = notTaken;
     526        notTaken = tmp;
     527    }
     528
     529    JSValueOperand arg1(this, node.child1());
     530    JSValueOperand arg2(this, node.child2());
     531    GPRReg arg1GPR = arg1.gpr();
     532    GPRReg arg2GPR = arg2.gpr();
     533   
     534    GPRTemporary result(this, arg2);
     535    GPRReg resultGPR = result.gpr();
     536   
     537    JITCompiler::JumpList slowPath;
     538   
     539    if (!isKnownInteger(node.child1()))
     540        slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg1GPR, GPRInfo::tagTypeNumberRegister));
     541    if (!isKnownInteger(node.child2()))
     542        slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg2GPR, GPRInfo::tagTypeNumberRegister));
     543   
     544    addBranch(m_jit.branch32(cond, arg1GPR, arg2GPR), taken);
     545   
     546    if (!isKnownInteger(node.child1()) || !isKnownInteger(node.child2())) {
     547        addBranch(m_jit.jump(), notTaken);
     548   
     549        slowPath.link(&m_jit);
     550   
     551        silentSpillAllRegisters(resultGPR);
     552        setupStubArguments(arg1GPR, arg2GPR);
     553        m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
     554        appendCallWithExceptionCheck(helperFunction);
     555        m_jit.move(GPRInfo::returnValueGPR, resultGPR);
     556        silentFillAllRegisters(resultGPR);
     557       
     558        addBranch(m_jit.branchTest8(callResultCondition, resultGPR), taken);
     559    }
     560
     561    if (notTaken != (m_block + 1))
     562        addBranch(m_jit.jump(), notTaken);
     563}
     564
     565bool JITCodeGenerator::nonSpeculativeCompare(Node& node, MacroAssembler::RelationalCondition cond, Z_DFGOperation_EJJ helperFunction)
     566{
     567    NodeIndex branchNodeIndex = detectPeepHoleBranch();
     568    if (branchNodeIndex != NoNode) {
     569        ASSERT(node.adjustedRefCount() == 1);
     570       
     571        nonSpeculativePeepholeBranch(node, branchNodeIndex, cond, helperFunction);
     572   
     573        use(node.child1());
     574        use(node.child2());
     575        m_compileIndex = branchNodeIndex;
     576       
     577        return true;
     578    }
     579       
     580    JSValueOperand arg1(this, node.child1());
     581    JSValueOperand arg2(this, node.child2());
     582    GPRReg arg1GPR = arg1.gpr();
     583    GPRReg arg2GPR = arg2.gpr();
     584   
     585    GPRTemporary result(this, arg2);
     586    GPRReg resultGPR = result.gpr();
     587   
     588    JITCompiler::JumpList slowPath;
     589   
     590    if (!isKnownInteger(node.child1()))
     591        slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg1GPR, GPRInfo::tagTypeNumberRegister));
     592    if (!isKnownInteger(node.child2()))
     593        slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg2GPR, GPRInfo::tagTypeNumberRegister));
     594   
     595    m_jit.compare32(cond, arg1GPR, arg2GPR, resultGPR);
     596   
     597    if (!isKnownInteger(node.child1()) || !isKnownInteger(node.child2())) {
     598        JITCompiler::Jump haveResult = m_jit.jump();
     599   
     600        slowPath.link(&m_jit);
     601       
     602        silentSpillAllRegisters(resultGPR);
     603        setupStubArguments(arg1GPR, arg2GPR);
     604        m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
     605        appendCallWithExceptionCheck(helperFunction);
     606        m_jit.move(GPRInfo::returnValueGPR, resultGPR);
     607        silentFillAllRegisters(resultGPR);
     608       
     609        m_jit.andPtr(TrustedImm32(1), resultGPR);
     610       
     611        haveResult.link(&m_jit);
     612    }
     613   
     614    m_jit.or32(TrustedImm32(ValueFalse), resultGPR);
     615   
     616    jsValueResult(resultGPR, m_compileIndex);
     617   
     618    return false;
    469619}
    470620
  • trunk/Source/JavaScriptCore/dfg/DFGJITCodeGenerator.h

    r91041 r91099  
    413413    }
    414414
     415    bool isKnownInteger(NodeIndex);
     416    bool isKnownNumeric(NodeIndex);
     417
    415418    // Checks/accessors for constant values.
    416419    bool isConstant(NodeIndex nodeIndex) { return m_jit.isConstant(nodeIndex); }
     
    532535    }
    533536   
     537    // Returns the node index of the branch node if peephole is okay, NoNode otherwise.
     538    NodeIndex detectPeepHoleBranch()
     539    {
     540        NodeIndex lastNodeIndex = m_jit.graph().m_blocks[m_block]->end - 1;
     541
     542        // Check that no intervening nodes will be generated.
     543        for (NodeIndex index = m_compileIndex + 1; index < lastNodeIndex; ++index) {
     544            if (m_jit.graph()[index].shouldGenerate())
     545                return NoNode;
     546        }
     547
     548        // Check if the lastNode is a branch on this node.
     549        Node& lastNode = m_jit.graph()[lastNodeIndex];
     550        return lastNode.op == Branch && lastNode.child1() == m_compileIndex ? lastNodeIndex : NoNode;
     551    }
     552
    534553    JITCompiler::Call cachedGetById(GPRReg baseGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget = JITCompiler::Jump(), NodeType = GetById);
    535554    void cachedPutById(GPRReg baseGPR, GPRReg valueGPR, GPRReg scratchGPR, unsigned identifierNumber, PutKind, JITCompiler::Jump slowPathTarget = JITCompiler::Jump());
    536555    void cachedGetMethod(GPRReg baseGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget = JITCompiler::Jump());
     556   
     557    void nonSpeculativePeepholeBranch(Node&, NodeIndex branchNodeIndex, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ helperFunction);
     558    bool nonSpeculativeCompare(Node&, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ helperFunction);
    537559   
    538560    void emitBranch(Node&);
  • trunk/Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp

    r91041 r91099  
    121121}
    122122
    123 bool NonSpeculativeJIT::isKnownInteger(NodeIndex nodeIndex)
    124 {
    125     if (isInt32Constant(nodeIndex))
    126         return true;
    127 
    128     GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
    129 
    130     DataFormat registerFormat = info.registerFormat();
    131     if (registerFormat != DataFormatNone)
    132         return (registerFormat | DataFormatJS) == DataFormatJSInteger;
    133 
    134     DataFormat spillFormat = info.spillFormat();
    135     if (spillFormat != DataFormatNone)
    136         return (spillFormat | DataFormatJS) == DataFormatJSInteger;
    137 
    138     ASSERT(isConstant(nodeIndex));
    139     return false;
    140 }
    141 
    142 bool NonSpeculativeJIT::isKnownNumeric(NodeIndex nodeIndex)
    143 {
    144     if (isInt32Constant(nodeIndex) || isDoubleConstant(nodeIndex))
    145         return true;
    146 
    147     GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
    148 
    149     DataFormat registerFormat = info.registerFormat();
    150     if (registerFormat != DataFormatNone)
    151         return (registerFormat | DataFormatJS) == DataFormatJSInteger
    152             || (registerFormat | DataFormatJS) == DataFormatJSDouble;
    153 
    154     DataFormat spillFormat = info.spillFormat();
    155     if (spillFormat != DataFormatNone)
    156         return (spillFormat | DataFormatJS) == DataFormatJSInteger
    157             || (spillFormat | DataFormatJS) == DataFormatJSDouble;
    158 
    159     ASSERT(isConstant(nodeIndex));
    160     return false;
    161 }
    162 
    163123void NonSpeculativeJIT::knownConstantArithOp(NodeType op, NodeIndex regChild, NodeIndex immChild, bool commute)
    164124{
     
    343303}
    344304
    345 void NonSpeculativeJIT::compare(Node& node, MacroAssembler::RelationalCondition cond, Z_DFGOperation_EJJ helperFunction)
    346 {
    347     // FIXME: should do some peephole to fuse compare/branch
    348        
    349     JSValueOperand arg1(this, node.child1());
    350     JSValueOperand arg2(this, node.child2());
    351     GPRReg arg1GPR = arg1.gpr();
    352     GPRReg arg2GPR = arg2.gpr();
    353    
    354     GPRTemporary result(this, arg2);
    355     GPRReg resultGPR = result.gpr();
    356    
    357     JITCompiler::JumpList slowPath;
    358    
    359     if (!isKnownInteger(node.child1()))
    360         slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg1GPR, GPRInfo::tagTypeNumberRegister));
    361     if (!isKnownInteger(node.child2()))
    362         slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg2GPR, GPRInfo::tagTypeNumberRegister));
    363    
    364     m_jit.compare32(cond, arg1GPR, arg2GPR, resultGPR);
    365    
    366     JITCompiler::Jump haveResult = m_jit.jump();
    367    
    368     slowPath.link(&m_jit);
    369    
    370     silentSpillAllRegisters(resultGPR);
    371     setupStubArguments(arg1GPR, arg2GPR);
    372     m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
    373     appendCallWithExceptionCheck(helperFunction);
    374     m_jit.move(GPRInfo::returnValueGPR, resultGPR);
    375     silentFillAllRegisters(resultGPR);
    376        
    377     m_jit.andPtr(TrustedImm32(static_cast<int32_t>(1)), resultGPR);
    378    
    379     haveResult.link(&m_jit);
    380    
    381     m_jit.or32(TrustedImm32(ValueFalse), resultGPR);
    382     jsValueResult(resultGPR, m_compileIndex);
    383 }
    384 
    385305void NonSpeculativeJIT::compile(SpeculationCheckIndexIterator& checkIterator, Node& node)
    386306{
     
    734654
    735655    case CompareLess:
    736         compare(node, MacroAssembler::LessThan, operationCompareLess);
     656        if (nonSpeculativeCompare(node, MacroAssembler::LessThan, operationCompareLess))
     657            return;
    737658        break;
    738659       
    739660    case CompareLessEq:
    740         compare(node, MacroAssembler::LessThanOrEqual, operationCompareLessEq);
     661        if (nonSpeculativeCompare(node, MacroAssembler::LessThanOrEqual, operationCompareLessEq))
     662            return;
    741663        break;
    742664       
    743665    case CompareGreater:
    744         compare(node, MacroAssembler::GreaterThan, operationCompareGreater);
     666        if (nonSpeculativeCompare(node, MacroAssembler::GreaterThan, operationCompareGreater))
     667            return;
    745668        break;
    746669       
    747670    case CompareGreaterEq:
    748         compare(node, MacroAssembler::GreaterThanOrEqual, operationCompareGreaterEq);
     671        if (nonSpeculativeCompare(node, MacroAssembler::GreaterThanOrEqual, operationCompareGreaterEq))
     672            return;
    749673        break;
    750674       
    751675    case CompareEq:
    752         compare(node, MacroAssembler::Equal, operationCompareEq);
     676        if (nonSpeculativeCompare(node, MacroAssembler::Equal, operationCompareEq))
     677            return;
    753678        break;
    754679
    755680    case CompareStrictEq:
    756         compare(node, MacroAssembler::Equal, operationCompareStrictEq);
     681        if (nonSpeculativeCompare(node, MacroAssembler::Equal, operationCompareStrictEq))
     682            return;
    757683        break;
    758684
  • trunk/Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.h

    r90371 r91099  
    8383    void compile(SpeculationCheckIndexIterator&, BasicBlock&);
    8484
    85     bool isKnownInteger(NodeIndex);
    86     bool isKnownNumeric(NodeIndex);
    87 
    8885    // These methods are used to plant calls out to C++
    8986    // helper routines to convert between types.
     
    103100    void knownConstantArithOp(NodeType op, NodeIndex regChild, NodeIndex immChild, bool commute);
    104101    void basicArithOp(NodeType op, Node&);
    105     void compare(Node&, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ);
    106102
    107103    EntryLocationVector m_entryLocations;
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r91095 r91099  
    425425            compilePeepHoleIntegerBranch(node, branchNodeIndex, condition);
    426426        else
    427             compilePeepHoleCall(node, branchNodeIndex, operation);
     427            nonSpeculativePeepholeBranch(node, branchNodeIndex, condition, operation);
    428428
    429429        use(node.child1());
     
    798798        break;
    799799
    800     case CompareStrictEq: {
    801         SpeculateIntegerOperand op1(this, node.child1());
    802         SpeculateIntegerOperand op2(this, node.child2());
    803         GPRTemporary result(this, op1, op2);
    804 
    805         m_jit.compare32(JITCompiler::Equal, op1.gpr(), op2.gpr(), result.gpr());
    806 
    807         // If we add a DataFormatBool, we should use it here.
    808         m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
    809         jsValueResult(result.gpr(), m_compileIndex);
    810         break;
    811     }
     800    case CompareStrictEq:
     801        if (compare(node, JITCompiler::Equal, operationCompareStrictEq))
     802            return;
     803        break;
    812804
    813805    case GetByVal: {
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h

    r91022 r91099  
    140140    void initializeVariableTypes();
    141141
    142     // Returns the node index of the branch node if peephole is okay, NoNode otherwise.
    143     NodeIndex detectPeepHoleBranch()
    144     {
    145         NodeIndex lastNodeIndex = m_jit.graph().m_blocks[m_block]->end - 1;
    146 
    147         // Check that no intervening nodes will be generated.
    148         for (NodeIndex index = m_compileIndex + 1; index < lastNodeIndex; ++index) {
    149             if (m_jit.graph()[index].shouldGenerate())
    150                 return NoNode;
    151         }
    152 
    153         // Check if the lastNode is a branch on this node.
    154         Node& lastNode = m_jit.graph()[lastNodeIndex];
    155         return lastNode.op == Branch && lastNode.child1() == m_compileIndex ? lastNodeIndex : NoNode;
    156     }
    157 
    158142    bool isInteger(NodeIndex nodeIndex)
    159143    {
Note: See TracChangeset for help on using the changeset viewer.