Changeset 99148 in webkit


Ignore:
Timestamp:
Nov 3, 2011, 1:06:42 AM (14 years ago)
Author:
fpizlo@apple.com
Message:

Source/JavaScriptCore: DFG inlining breaks function.arguments[something] if the argument being
retrieved was subjected to DFG's unboxing optimizations
​https://bugs.webkit.org/show_bug.cgi?id=71436

Reviewed by Oliver Hunt.

This makes inlined arguments retrieval use some of the same machinery as
OSR to determine where from, and how, to retrieve a value that the DFG
might have somehow squirreled away while the old JIT would put it in its
obvious location, using an obvious format.

To that end, previously DFG-internal notions such as DataFormat,
VirtualRegister, and ValueRecovery are now in bytecode/ since they are
stored as part of InlineCallFrames.

  • bytecode/CodeOrigin.h:
  • dfg/DFGAbstractState.cpp:

(JSC::DFG::AbstractState::execute):

  • dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::handleInlining):
(JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):

  • dfg/DFGJITCompiler.cpp:

(JSC::DFG::JITCompiler::exitSpeculativeWithOSR):

  • dfg/DFGJITCompiler32_64.cpp:

(JSC::DFG::JITCompiler::exitSpeculativeWithOSR):

  • dfg/DFGNode.h:
  • dfg/DFGPropagator.cpp:

(JSC::DFG::Propagator::propagateNodePredictions):

  • dfg/DFGSpeculativeJIT.cpp:

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

  • dfg/DFGSpeculativeJIT64.cpp:

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

  • interpreter/CallFrame.cpp:

(JSC::CallFrame::trueCallerFrame):

  • interpreter/CallFrame.h:

(JSC::ExecState::inlineCallFrame):

  • interpreter/Register.h:

(JSC::Register::asInlineCallFrame):
(JSC::Register::unboxedInt32):
(JSC::Register::unboxedBoolean):
(JSC::Register::unboxedCell):

  • runtime/Arguments.h:

(JSC::Arguments::finishCreationAndCopyRegisters):

LayoutTests: DFG inlining breaks function.arguments[something] if the argument being
retrieved was subjected to DFG's unboxing optimizations
​https://bugs.webkit.org/show_bug.cgi?id=71436

Reviewed by Oliver Hunt.

  • fast/js/dfg-inline-arguments-int32-expected.txt: Added.
  • fast/js/dfg-inline-arguments-int32.html: Added.
  • fast/js/script-tests/dfg-inline-arguments-int32.js: Added.

(foo):
(bar):
(baz):
(argsToStr):

Location:
trunk
Files:
3 added
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r99143 r99148  
     12011-11-02  Filip Pizlo  <fpizlo@apple.com>
     2
     3        DFG inlining breaks function.arguments[something] if the argument being
     4        retrieved was subjected to DFG's unboxing optimizations
     5        https://bugs.webkit.org/show_bug.cgi?id=71436       
     6
     7        Reviewed by Oliver Hunt.
     8
     9        * fast/js/dfg-inline-arguments-int32-expected.txt: Added.
     10        * fast/js/dfg-inline-arguments-int32.html: Added.
     11        * fast/js/script-tests/dfg-inline-arguments-int32.js: Added.
     12        (foo):
     13        (bar):
     14        (baz):
     15        (argsToStr):
     16
    1172011-11-02  Adam Barth  <abarth@webkit.org>
    218

  • trunk/Source/JavaScriptCore/ChangeLog

    r99144 r99148  
     12011-11-02  Filip Pizlo  <fpizlo@apple.com>
     2
     3        DFG inlining breaks function.arguments[something] if the argument being
     4        retrieved was subjected to DFG's unboxing optimizations
     5        https://bugs.webkit.org/show_bug.cgi?id=71436
     6
     7        Reviewed by Oliver Hunt.
     8       
     9        This makes inlined arguments retrieval use some of the same machinery as
     10        OSR to determine where from, and how, to retrieve a value that the DFG
     11        might have somehow squirreled away while the old JIT would put it in its
     12        obvious location, using an obvious format.
     13       
     14        To that end, previously DFG-internal notions such as DataFormat,
     15        VirtualRegister, and ValueRecovery are now in bytecode/ since they are
     16        stored as part of InlineCallFrames.
     17
     18        * bytecode/CodeOrigin.h:
     19        * dfg/DFGAbstractState.cpp:
     20        (JSC::DFG::AbstractState::execute):
     21        * dfg/DFGByteCodeParser.cpp:
     22        (JSC::DFG::ByteCodeParser::handleInlining):
     23        (JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
     24        * dfg/DFGJITCompiler.cpp:
     25        (JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
     26        * dfg/DFGJITCompiler32_64.cpp:
     27        (JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
     28        * dfg/DFGNode.h:
     29        * dfg/DFGPropagator.cpp:
     30        (JSC::DFG::Propagator::propagateNodePredictions):
     31        * dfg/DFGSpeculativeJIT.cpp:
     32        (JSC::DFG::SpeculativeJIT::compile):
     33        * dfg/DFGSpeculativeJIT64.cpp:
     34        (JSC::DFG::SpeculativeJIT::compile):
     35        * interpreter/CallFrame.cpp:
     36        (JSC::CallFrame::trueCallerFrame):
     37        * interpreter/CallFrame.h:
     38        (JSC::ExecState::inlineCallFrame):
     39        * interpreter/Register.h:
     40        (JSC::Register::asInlineCallFrame):
     41        (JSC::Register::unboxedInt32):
     42        (JSC::Register::unboxedBoolean):
     43        (JSC::Register::unboxedCell):
     44        * runtime/Arguments.h:
     45        (JSC::Arguments::finishCreationAndCopyRegisters):
     46
    1472011-11-02  Filip Pizlo  <fpizlo@apple.com>
    248

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

    r98831 r99148  
    2727#define CodeOrigin_h
    2828
     29#include "ValueRecovery.h"
    2930#include "WriteBarrier.h"
    3031#include <wtf/StdLibExtras.h>
    … …  
    7677
    7778struct InlineCallFrame {
     79    Vector<ValueRecovery> arguments;
    7880    WriteBarrier<ExecutableBase> executable;
    7981    WriteBarrier<JSFunction> callee;
    8082    CodeOrigin caller;
    81     unsigned stackOffset;
    82     unsigned numArgumentsIncludingThis : 31;
     83    unsigned stackOffset : 31;
    8384    bool isCall : 1;
    8485};

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

    r98912 r99148  
    660660           
    661661    case Phantom:
     662    case InlineStart:
    662663        break;
    663664    }

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

    r98912 r99148  
    10041004    unsigned oldIndex = m_currentIndex;
    10051005    m_currentIndex = 0;
     1006
     1007    addToGraph(InlineStart);
     1008   
    10061009    parseCodeBlock();
     1010   
    10071011    m_currentIndex = oldIndex;
    10081012   
    … …  
    23272331        inlineCallFrame.callee.set(*byteCodeParser->m_globalData, byteCodeParser->m_codeBlock->ownerExecutable(), callee);
    23282332        inlineCallFrame.caller = byteCodeParser->currentCodeOrigin();
    2329         inlineCallFrame.numArgumentsIncludingThis = codeBlock->m_numParameters;
     2333        inlineCallFrame.arguments.resize(codeBlock->m_numParameters); // Set the number of arguments including this, but don't configure the value recoveries, yet.
    23302334        inlineCallFrame.isCall = isCall(kind);
    23312335        byteCodeParser->m_codeBlock->inlineCallFrames().append(inlineCallFrame);

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

    r99009 r99148  
    784784        storePtr(callerFrameGPR, addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::CallerFrame)));
    785785        storePtr(TrustedImmPtr(jumpTarget), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ReturnPC)));
    786         storePtr(TrustedImmPtr(JSValue::encode(jsNumber(inlineCallFrame->numArgumentsIncludingThis))), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
     786        storePtr(TrustedImmPtr(JSValue::encode(jsNumber(inlineCallFrame->arguments.size()))), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
    787787        storePtr(TrustedImmPtr(inlineCallFrame->callee.get()), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::Callee)));
    788788    }

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

    r99129 r99148  
    529529        storePtr(TrustedImmPtr(jumpTarget), payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ReturnPC)));
    530530        store32(Imm32(JSValue::Int32Tag), tagFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
    531         store32(Imm32(inlineCallFrame->numArgumentsIncludingThis), payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
     531        store32(Imm32(inlineCallFrame->arguments.size()), payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
    532532        store32(Imm32(JSValue::CellTag), tagFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::Callee)));
    533533        storePtr(TrustedImmPtr(inlineCallFrame->callee.get()), payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::Callee)));

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

    r99009 r99148  
    221221    macro(SetArgument, 0) \
    222222    \
     223    /* Hint that inlining begins here. No code is generated for this node. It's only */\
     224    /* used for copying OSR data into inline frame data, to support reification of */\
     225    /* call frames of inlined functions. */\
     226    macro(InlineStart, 0) \
     227    \
    223228    /* Nodes for bitwise operations. */\
    224229    macro(BitAnd, NodeResultInt32) \

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

    r98912 r99148  
    608608            break;
    609609           
    610         // This gets ignored because it doesn't do anything.
     610        // These gets ignored because it doesn't do anything.
    611611        case Phantom:
     612        case InlineStart:
    612613            break;
    613614#else

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

    r99144 r99148  
    264264            fprintf(stderr, "SpeculativeJIT skipping Node @%d (bc#%u) at JIT offset 0x%x     ", (int)m_compileIndex, node.codeOrigin.bytecodeIndex, m_jit.debugOffset());
    265265#endif
    266             if (node.op == SetLocal)
     266            switch (node.op) {
     267            case SetLocal:
    267268                compileMovHint(node);
     269                break;
     270
     271            case InlineStart: {
     272                InlineCallFrame* inlineCallFrame = node.codeOrigin.inlineCallFrame;
     273                unsigned argumentsStart = inlineCallFrame->stackOffset - RegisterFile::CallFrameHeaderSize - inlineCallFrame->arguments.size();
     274                for (unsigned i = 0; i < inlineCallFrame->arguments.size(); ++i) {
     275                    ValueRecovery recovery = computeValueRecoveryFor(m_variables[argumentsStart + i]);
     276                    // The recovery cannot point to registers, since the call frame reification isn't
     277                    // as smart as OSR, so it can't handle that. The exception is the this argument,
     278                    // which we don't really need to be able to recover.
     279                    ASSERT(!i || !recovery.isInRegisters());
     280                    inlineCallFrame->arguments[i] = recovery;
     281                }
     282                break;
     283            }
     284               
     285            default:
     286                break;
     287            }
    268288        } else {
    269289           

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

    r99132 r99148  
    24922492        noResult(m_compileIndex);
    24932493        break;
     2494       
     2495    case InlineStart:
     2496        ASSERT_NOT_REACHED();
     2497        break;
    24942498    }
    24952499

  • trunk/Source/JavaScriptCore/interpreter/CallFrame.cpp

    r99009 r99148  
    111111       
    112112        inlinedCaller->setInlineCallFrame(inlineCallFrame);
    113         inlinedCaller->setArgumentCountIncludingThis(inlineCallFrame->numArgumentsIncludingThis);
     113        inlinedCaller->setArgumentCountIncludingThis(inlineCallFrame->arguments.size());
    114114        inlinedCaller->setCallee(calleeAsFunction);
    115115       

  • trunk/Source/JavaScriptCore/interpreter/CallFrame.h

    r99009 r99148  
    106106#endif
    107107#if ENABLE(DFG_JIT)
    108         InlineCallFrame* inlineCallFrame() const { return this[RegisterFile::ReturnPC].inlineCallFrame(); }
     108        InlineCallFrame* inlineCallFrame() const { return this[RegisterFile::ReturnPC].asInlineCallFrame(); }
     109#else
     110        // This will never be called if !ENABLE(DFG_JIT) since all calls should be guarded by
     111        // isInlineCallFrame(). But to make it easier to write code without having a bunch of
     112        // #if's, we make a dummy implementation available anyway.
     113        InlineCallFrame* inlineCallFrame() const
     114        {
     115            ASSERT_NOT_REACHED();
     116            return 0;
     117        }
    109118#endif
    110119#if ENABLE(INTERPRETER)

  • trunk/Source/JavaScriptCore/interpreter/Register.h

    r97512 r99148  
    7272        ScopeChainNode* scopeChain() const;
    7373        Instruction* vPC() const;
    74         InlineCallFrame* inlineCallFrame() const;
     74        InlineCallFrame* asInlineCallFrame() const;
     75        int32_t unboxedInt32() const;
     76        bool unboxedBoolean() const;
     77        JSCell* unboxedCell() const;
    7578
    7679        static Register withInt(int32_t i)
    … …  
    8992            Instruction* vPC;
    9093            InlineCallFrame* inlineCallFrame;
     94            EncodedValueDescriptor encodedValue;
    9195        } u;
    9296    };
    … …  
    166170    }
    167171
    168     ALWAYS_INLINE InlineCallFrame* Register::inlineCallFrame() const
     172    ALWAYS_INLINE InlineCallFrame* Register::asInlineCallFrame() const
    169173    {
    170174        return u.inlineCallFrame;
    171175    }
     176       
     177    ALWAYS_INLINE int32_t Register::unboxedInt32() const
     178    {
     179        return u.encodedValue.asBits.payload;
     180    }
     181
     182    ALWAYS_INLINE bool Register::unboxedBoolean() const
     183    {
     184        return !!u.encodedValue.asBits.payload;
     185    }
     186
     187    ALWAYS_INLINE JSCell* Register::unboxedCell() const
     188    {
     189#if USE(JSVALUE64)
     190        return u.encodedValue.ptr;
     191#else
     192        return bitwise_cast<JSCell*>(u.encodedValue.asBits.payload);
     193#endif
     194    }
    172195
    173196} // namespace JSC

  • trunk/Source/JavaScriptCore/runtime/Arguments.h

    r99126 r99148  
    260260           
    261261            OwnArrayPtr<WriteBarrier<Unknown> > registerArray = adoptArrayPtr(new WriteBarrier<Unknown>[registerArraySize]);
    262             for (size_t i = 0; i < registerArraySize; ++i)
    263                 registerArray[i].set(callFrame->globalData(), this, callFrame->registers()[i - registerOffset].jsValue());
     262            if (callFrame->isInlineCallFrame()) {
     263                InlineCallFrame* inlineCallFrame = callFrame->inlineCallFrame();
     264                for (size_t i = 0; i < registerArraySize; ++i) {
     265                    ValueRecovery& recovery = inlineCallFrame->arguments[i + 1];
     266                    // In the future we'll support displaced recoveries (indicating that the
     267                    // argument was flushed to a different location), but for now we don't do
     268                    // that so this code will fail if that were to happen. On the other hand,
     269                    // it's much less likely that we'll support in-register recoveries since
     270                    // this code does not (easily) have access to registers.
     271                    JSValue value;
     272                    Register* location = callFrame->registers() + i - registerOffset;
     273                    switch (recovery.technique()) {
     274                    case AlreadyInRegisterFile:
     275                        value = location->jsValue();
     276                        break;
     277                    case AlreadyInRegisterFileAsUnboxedInt32:
     278                        value = jsNumber(location->unboxedInt32());
     279                        break;
     280                    case AlreadyInRegisterFileAsUnboxedCell:
     281                        value = location->unboxedCell();
     282                        break;
     283                    case AlreadyInRegisterFileAsUnboxedBoolean:
     284                        value = jsBoolean(location->unboxedBoolean());
     285                        break;
     286                    case Constant:
     287                        value = recovery.constant();
     288                        break;
     289                    default:
     290                        ASSERT_NOT_REACHED();
     291                        break;
     292                    }
     293                    registerArray[i].set(callFrame->globalData(), this, value);
     294                }
     295            } else {
     296                for (size_t i = 0; i < registerArraySize; ++i)
     297                    registerArray[i].set(callFrame->globalData(), this, callFrame->registers()[i - registerOffset].jsValue());
     298            }
    264299            d->registers = registerArray.get() + d->numParameters + RegisterFile::CallFrameHeaderSize;
    265300            d->registerArray = registerArray.release();
Note: See TracChangeset for help on using the changeset viewer.