Changeset 156468 in webkit


Ignore:
Timestamp:
Sep 26, 2013 10:16:47 AM (11 years ago)
Author:
mhahnenberg@apple.com
Message:

op_to_this shouldn't use value profiling
​https://bugs.webkit.org/show_bug.cgi?id=121920

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Currently it's the only opcode that uses m_singletonValue, which is unnecessary. Our current plan is
to remove m_singletonValue so that GenGC can have a simpler story for handling CodeBlocks/FunctionExecutables
during nursery collections.

This patch adds an inline cache for the Structure of to_this so it no longer depends on the ValueProfile's
m_singletonValue. Since nobody uses m_singletonValue now, this patch also removes m_singletonValue from
ValueProfile.

  • bytecode/CodeBlock.cpp:

(JSC::CodeBlock::CodeBlock):
(JSC::CodeBlock::finalizeUnconditionally):
(JSC::CodeBlock::stronglyVisitStrongReferences):
(JSC::CodeBlock::updateAllPredictionsAndCountLiveness):
(JSC::CodeBlock::updateAllValueProfilePredictions):
(JSC::CodeBlock::updateAllPredictions):
(JSC::CodeBlock::shouldOptimizeNow):

  • bytecode/CodeBlock.h:

(JSC::CodeBlock::updateAllValueProfilePredictions):
(JSC::CodeBlock::updateAllPredictions):

  • bytecode/LazyOperandValueProfile.cpp:

(JSC::CompressedLazyOperandValueProfileHolder::computeUpdatedPredictions):

  • bytecode/LazyOperandValueProfile.h:
  • bytecode/ValueProfile.h:

(JSC::ValueProfileBase::ValueProfileBase):
(JSC::ValueProfileBase::briefDescription):
(JSC::ValueProfileBase::dump):
(JSC::ValueProfileBase::computeUpdatedPrediction):

  • bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::BytecodeGenerator):

  • dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::parseBlock):

  • jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_to_this):
(JSC::JIT::emitSlow_op_to_this):

  • jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_to_this):
(JSC::JIT::emitSlow_op_to_this):

  • llint/LowLevelInterpreter32_64.asm:
  • llint/LowLevelInterpreter64.asm:
  • runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

LayoutTests:

Updated a couple tests that waited for two DFG compiles, but with this patch we
don't do two compiles any more, so we don't want to wait forever.

  • js/script-tests/dfg-convert-this-polymorphic-object-then-exit-on-other.js:
  • js/script-tests/dfg-convert-this-polymorphic-object-then-exit-on-string.js:
Location:
trunk
Files:
16 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r156467 r156468  
     12013-09-26  Mark Hahnenberg  <mhahnenberg@apple.com>
     2
     3        op_to_this shouldn't use value profiling
     4        https://bugs.webkit.org/show_bug.cgi?id=121920
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Updated a couple tests that waited for two DFG compiles, but with this patch we
     9        don't do two compiles any more, so we don't want to wait forever.
     10
     11        * js/script-tests/dfg-convert-this-polymorphic-object-then-exit-on-other.js:
     12        * js/script-tests/dfg-convert-this-polymorphic-object-then-exit-on-string.js:
     13
    1142013-09-26  Andreas Kling  <akling@apple.com>
    215

  • trunk/LayoutTests/js/script-tests/dfg-convert-this-polymorphic-object-then-exit-on-other.js

    r155201 r156468  
    1212noInline(foo);
    1313
    14 for (var i = 0; i < 1000; i = dfgIncrement({f:foo, i:dfgIncrement({f:foo, i:i + 1, n:100}), n:500, compiles:2})) {
     14for (var i = 0; i < 1000; i = dfgIncrement({f:foo, i:i + 1, n:500})) {
    1515    var me;
    1616    if (i < 150)

  • trunk/LayoutTests/js/script-tests/dfg-convert-this-polymorphic-object-then-exit-on-string.js

    r155201 r156468  
    1414noInline(foo);
    1515
    16 for (var i = 0; i < 1000; i = dfgIncrement({f:foo, i:dfgIncrement({f:foo, i:i + 1, n:100}), n:500, compiles:2})) {
     16for (var i = 0; i < 1000; i = dfgIncrement({f:foo, i:i + 1, n:500})) {
    1717    var me;
    1818    if (i < 150)

  • trunk/Source/JavaScriptCore/ChangeLog

    r156464 r156468  
     12013-09-26  Mark Hahnenberg  <mhahnenberg@apple.com>
     2
     3        op_to_this shouldn't use value profiling
     4        https://bugs.webkit.org/show_bug.cgi?id=121920
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Currently it's the only opcode that uses m_singletonValue, which is unnecessary. Our current plan is
     9        to remove m_singletonValue so that GenGC can have a simpler story for handling CodeBlocks/FunctionExecutables
     10        during nursery collections.
     11
     12        This patch adds an inline cache for the Structure of to_this so it no longer depends on the ValueProfile's
     13        m_singletonValue. Since nobody uses m_singletonValue now, this patch also removes m_singletonValue from
     14        ValueProfile.
     15
     16        * bytecode/CodeBlock.cpp:
     17        (JSC::CodeBlock::CodeBlock):
     18        (JSC::CodeBlock::finalizeUnconditionally):
     19        (JSC::CodeBlock::stronglyVisitStrongReferences):
     20        (JSC::CodeBlock::updateAllPredictionsAndCountLiveness):
     21        (JSC::CodeBlock::updateAllValueProfilePredictions):
     22        (JSC::CodeBlock::updateAllPredictions):
     23        (JSC::CodeBlock::shouldOptimizeNow):
     24        * bytecode/CodeBlock.h:
     25        (JSC::CodeBlock::updateAllValueProfilePredictions):
     26        (JSC::CodeBlock::updateAllPredictions):
     27        * bytecode/LazyOperandValueProfile.cpp:
     28        (JSC::CompressedLazyOperandValueProfileHolder::computeUpdatedPredictions):
     29        * bytecode/LazyOperandValueProfile.h:
     30        * bytecode/ValueProfile.h:
     31        (JSC::ValueProfileBase::ValueProfileBase):
     32        (JSC::ValueProfileBase::briefDescription):
     33        (JSC::ValueProfileBase::dump):
     34        (JSC::ValueProfileBase::computeUpdatedPrediction):
     35        * bytecompiler/BytecodeGenerator.cpp:
     36        (JSC::BytecodeGenerator::BytecodeGenerator):
     37        * dfg/DFGByteCodeParser.cpp:
     38        (JSC::DFG::ByteCodeParser::parseBlock):
     39        * jit/JITOpcodes.cpp:
     40        (JSC::JIT::emit_op_to_this):
     41        (JSC::JIT::emitSlow_op_to_this):
     42        * jit/JITOpcodes32_64.cpp:
     43        (JSC::JIT::emit_op_to_this):
     44        (JSC::JIT::emitSlow_op_to_this):
     45        * llint/LowLevelInterpreter32_64.asm:
     46        * llint/LowLevelInterpreter64.asm:
     47        * runtime/CommonSlowPaths.cpp:
     48        (JSC::SLOW_PATH_DECL):
     49
    1502013-09-25  Oliver Hunt  <oliver@apple.com>
    251

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

    r156376 r156468  
    17351735            // fallthrough
    17361736        }
    1737         case op_to_this:
    17381737        case op_get_by_id:
    17391738        case op_call_varargs: {
    … …  
    22392238            case op_get_array_length:
    22402239                break;
     2240            case op_to_this:
     2241                if (!curInstruction[2].u.structure || Heap::isMarked(curInstruction[2].u.structure.get()))
     2242                    break;
     2243                if (Options::verboseOSR())
     2244                    dataLogF("Clearing LLInt to_this with structure %p.\n", curInstruction[2].u.structure.get());
     2245                curInstruction[2].u.structure.clear();
     2246                break;
    22412247            case op_get_callee:
    22422248                if (!curInstruction[2].u.jsCell || Heap::isMarked(curInstruction[2].u.jsCell.get()))
    … …  
    24192425        m_objectAllocationProfiles[i].visitAggregate(visitor);
    24202426
    2421     updateAllPredictions(Collection);
     2427    updateAllPredictions();
    24222428}
    24232429
    … …  
    30943100}
    30953101
    3096 void CodeBlock::updateAllPredictionsAndCountLiveness(
    3097     HeapOperation operation, unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles)
     3102void CodeBlock::updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles)
    30983103{
    30993104    ConcurrentJITLocker locker(m_lock);
    … …  
    31083113        numberOfSamplesInProfiles += numSamples;
    31093114        if (profile->m_bytecodeOffset < 0) {
    3110             profile->computeUpdatedPrediction(locker, operation);
     3115            profile->computeUpdatedPrediction(locker);
    31113116            continue;
    31123117        }
    31133118        if (profile->numberOfSamples() || profile->m_prediction != SpecNone)
    31143119            numberOfLiveNonArgumentValueProfiles++;
    3115         profile->computeUpdatedPrediction(locker, operation);
     3120        profile->computeUpdatedPrediction(locker);
    31163121    }
    31173122   
    31183123#if ENABLE(DFG_JIT)
    3119     m_lazyOperandValueProfiles.computeUpdatedPredictions(locker, operation);
    3120 #endif
    3121 }
    3122 
    3123 void CodeBlock::updateAllValueProfilePredictions(HeapOperation operation)
     3124    m_lazyOperandValueProfiles.computeUpdatedPredictions(locker);
     3125#endif
     3126}
     3127
     3128void CodeBlock::updateAllValueProfilePredictions()
    31243129{
    31253130    unsigned ignoredValue1, ignoredValue2;
    3126     updateAllPredictionsAndCountLiveness(operation, ignoredValue1, ignoredValue2);
     3131    updateAllPredictionsAndCountLiveness(ignoredValue1, ignoredValue2);
    31273132}
    31283133
    … …  
    31393144}
    31403145
    3141 void CodeBlock::updateAllPredictions(HeapOperation operation)
    3142 {
    3143     updateAllValueProfilePredictions(operation);
     3146void CodeBlock::updateAllPredictions()
     3147{
     3148    updateAllValueProfilePredictions();
    31443149    updateAllArrayPredictions();
    31453150}
    … …  
    31613166    unsigned numberOfLiveNonArgumentValueProfiles;
    31623167    unsigned numberOfSamplesInProfiles;
    3163     updateAllPredictionsAndCountLiveness(NoOperation, numberOfLiveNonArgumentValueProfiles, numberOfSamplesInProfiles);
     3168    updateAllPredictionsAndCountLiveness(numberOfLiveNonArgumentValueProfiles, numberOfSamplesInProfiles);
    31643169
    31653170    if (Options::verboseOSR()) {

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

    r156300 r156468  
    871871#if ENABLE(VALUE_PROFILER)
    872872    bool shouldOptimizeNow();
    873     void updateAllValueProfilePredictions(HeapOperation = NoOperation);
     873    void updateAllValueProfilePredictions();
    874874    void updateAllArrayPredictions();
    875     void updateAllPredictions(HeapOperation = NoOperation);
     875    void updateAllPredictions();
    876876#else
    877877    bool updateAllPredictionsAndCheckIfShouldOptimizeNow() { return false; }
    878     void updateAllValueProfilePredictions(HeapOperation = NoOperation) { }
     878    void updateAllValueProfilePredictions() { }
    879879    void updateAllArrayPredictions() { }
    880     void updateAllPredictions(HeapOperation = NoOperation) { }
     880    void updateAllPredictions() { }
    881881#endif
    882882
    … …  
    942942       
    943943#if ENABLE(VALUE_PROFILER)
    944     void updateAllPredictionsAndCountLiveness(HeapOperation, unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles);
     944    void updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles);
    945945#endif
    946946

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

    r156050 r156468  
    3636CompressedLazyOperandValueProfileHolder::~CompressedLazyOperandValueProfileHolder() { }
    3737
    38 void CompressedLazyOperandValueProfileHolder::computeUpdatedPredictions(const ConcurrentJITLocker& locker, HeapOperation operation)
     38void CompressedLazyOperandValueProfileHolder::computeUpdatedPredictions(const ConcurrentJITLocker& locker)
    3939{
    4040    if (!m_data)
    … …  
    4242   
    4343    for (unsigned i = 0; i < m_data->size(); ++i)
    44         m_data->at(i).computeUpdatedPrediction(locker, operation);
     44        m_data->at(i).computeUpdatedPrediction(locker);
    4545}
    4646

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

    r156050 r156468  
    158158    ~CompressedLazyOperandValueProfileHolder();
    159159   
    160     void computeUpdatedPredictions(const ConcurrentJITLocker&, HeapOperation);
     160    void computeUpdatedPredictions(const ConcurrentJITLocker&);
    161161   
    162162    LazyOperandValueProfile* add(

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

    r156050 r156468  
    5656        , m_prediction(SpecNone)
    5757        , m_numberOfSamplesInPrediction(0)
    58         , m_singletonValueIsTop(false)
    5958    {
    6059        for (unsigned i = 0; i < totalNumberOfBuckets; ++i)
    … …  
    6665        , m_prediction(SpecNone)
    6766        , m_numberOfSamplesInPrediction(0)
    68         , m_singletonValueIsTop(false)
    6967    {
    7068        for (unsigned i = 0; i < totalNumberOfBuckets; ++i)
    … …  
    118116       
    119117        StringPrintStream out;
    120        
    121         if (m_singletonValueIsTop)
    122             out.print("predicting ", SpeculationDump(m_prediction));
    123         else if (m_singletonValue)
    124             out.print("predicting ", m_singletonValue);
    125        
     118        out.print("predicting ", SpeculationDump(m_prediction));
    126119        return out.toCString();
    127120    }
    … …  
    130123    {
    131124        out.print("samples = ", totalNumberOfSamples(), " prediction = ", SpeculationDump(m_prediction));
    132         out.printf(", value = ");
    133         if (m_singletonValueIsTop)
    134             out.printf("TOP");
    135         else
    136             out.print(m_singletonValue);
    137125        bool first = true;
    138126        for (unsigned i = 0; i < totalNumberOfBuckets; ++i) {
    … …  
    151139    // Updates the prediction and returns the new one. Never call this from any thread
    152140    // that isn't executing the code.
    153     SpeculatedType computeUpdatedPrediction(const ConcurrentJITLocker&, HeapOperation operation = NoOperation)
     141    SpeculatedType computeUpdatedPrediction(const ConcurrentJITLocker&)
    154142    {
    155143        for (unsigned i = 0; i < totalNumberOfBuckets; ++i) {
    … …  
    161149            mergeSpeculation(m_prediction, speculationFromValue(value));
    162150           
    163             if (!m_singletonValueIsTop && !!value) {
    164                 if (!m_singletonValue)
    165                     m_singletonValue = value;
    166                 else if (m_singletonValue != value)
    167                     m_singletonValueIsTop = true;
    168             }
    169            
    170151            m_buckets[i] = JSValue::encode(JSValue());
    171152        }
    172153       
    173         if (operation == Collection
    174             && !m_singletonValueIsTop
    175             && !!m_singletonValue
    176             && m_singletonValue.isCell()
    177             && !Heap::isMarked(m_singletonValue.asCell()))
    178             m_singletonValueIsTop = true;
    179            
    180154        return m_prediction;
    181155    }
    … …  
    186160    unsigned m_numberOfSamplesInPrediction;
    187161   
    188     bool m_singletonValueIsTop;
    189     JSValue m_singletonValue;
    190 
    191162    EncodedJSValue m_buckets[totalNumberOfBuckets];
    192163};

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r156464 r156468  
    388388        emitCreateThis(&m_thisRegister);
    389389    } else if (functionBody->usesThis() || codeBlock->usesEval() || m_shouldEmitDebugHooks) {
    390         UnlinkedValueProfile profile = emitProfiledOpcode(op_to_this);
     390        emitOpcode(op_to_this);
    391391        instructions().append(kill(&m_thisRegister));
    392         instructions().append(profile);
     392        instructions().append(0);
    393393    }
    394394    for (size_t i = 0; i < deconstructedParameters.size(); i++) {

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

    r156376 r156468  
    19061906            Node* op1 = getThis();
    19071907            if (op1->op() != ToThis) {
    1908                 ConcurrentJITLocker locker(m_inlineStackTop->m_profiledBlock->m_lock);
    1909                 ValueProfile* profile =
    1910                     m_inlineStackTop->m_profiledBlock->valueProfileForBytecodeOffset(m_currentIndex);
    1911                 profile->computeUpdatedPrediction(locker);
    1912 #if DFG_ENABLE(DEBUG_VERBOSE)
    1913                 dataLogF("[bc#%u]: profile %p: ", m_currentIndex, profile);
    1914                 profile->dump(WTF::dataFile());
    1915                 dataLogF("\n");
    1916 #endif
    1917                 if (profile->m_singletonValueIsTop
    1918                     || !profile->m_singletonValue
    1919                     || !profile->m_singletonValue.isCell()
    1920                     || profile->m_singletonValue.asCell()->classInfo() != Structure::info()
    1921                     || static_cast<Structure*>(profile->m_singletonValue.asCell())->classInfo()->methodTable.toThis != JSObject::info()->methodTable.toThis)
     1908                Structure* cachedStructure = currentInstruction[2].u.structure.get();
     1909                if (!cachedStructure
     1910                    || cachedStructure->classInfo()->methodTable.toThis != JSObject::info()->methodTable.toThis
     1911                    || m_inlineStackTop->m_profiledBlock->couldTakeSlowCase(m_currentIndex)
     1912                    || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCache)) {
    19221913                    setThis(addToGraph(ToThis, op1));
    1923                 else {
     1914                } else {
    19241915                    addToGraph(
    19251916                        CheckStructure,
    1926                         OpInfo(m_graph.addStructureSet(jsCast<Structure*>(profile->m_singletonValue.asCell()))),
     1917                        OpInfo(m_graph.addStructureSet(cachedStructure)),
    19271918                        op1);
    19281919                }

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

    r156376 r156468  
    866866void JIT::emit_op_to_this(Instruction* currentInstruction)
    867867{
     868    WriteBarrierBase<Structure>* cachedStructure = &currentInstruction[2].u.structure;
    868869    emitGetVirtualRegister(currentInstruction[1].u.operand, regT1);
    869870
    870871    emitJumpSlowCaseIfNotJSCell(regT1);
    871872    loadPtr(Address(regT1, JSCell::structureOffset()), regT0);
    872     if (shouldEmitProfiling())
    873         emitValueProfilingSite(regT4);
    874873
    875874    addSlowCase(branch8(NotEqual, Address(regT0, Structure::typeInfoTypeOffset()), TrustedImm32(FinalObjectType)));
     875    loadPtr(cachedStructure, regT2);
     876    addSlowCase(branchPtr(NotEqual, regT0, regT2));
    876877}
    877878
    … …  
    944945void JIT::emitSlow_op_to_this(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
    945946{
     947    linkSlowCase(iter);
    946948    linkSlowCase(iter);
    947949    linkSlowCase(iter);

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

    r156376 r156468  
    11811181void JIT::emit_op_to_this(Instruction* currentInstruction)
    11821182{
     1183    WriteBarrierBase<Structure>* cachedStructure = &currentInstruction[2].u.structure;
    11831184    int thisRegister = currentInstruction[1].u.operand;
    11841185
    … …  
    11871188    addSlowCase(branch32(NotEqual, regT3, TrustedImm32(JSValue::CellTag)));
    11881189    loadPtr(Address(regT2, JSCell::structureOffset()), regT0);
    1189     if (shouldEmitProfiling()) {
    1190         move(regT3, regT1);
    1191         emitValueProfilingSite(regT4);
    1192     }
    11931190    addSlowCase(branch8(NotEqual, Address(regT0, Structure::typeInfoTypeOffset()), TrustedImm32(FinalObjectType)));
     1191    loadPtr(cachedStructure, regT2);
     1192    addSlowCase(branchPtr(NotEqual, regT0, regT2));
    11941193}
    11951194
    … …  
    11971196{
    11981197    int dst = currentInstruction[1].u.operand;
     1198    linkSlowCase(iter);
    11991199    linkSlowCase(iter);
    12001200    linkSlowCase(iter);

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

    r156376 r156468  
    432432    loadp JSCell::m_structure[t0], t0
    433433    bbneq Structure::m_typeInfo + TypeInfo::m_type[t0], FinalObjectType, .opToThisSlow
    434     valueProfile(CellTag, t0, 8, t1)
     434    loadpFromInstruction(2, t2)
     435    bpneq t0, t2, .opToThisSlow
    435436    dispatch(3)
    436437

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

    r156376 r156468  
    312312    loadp JSCell::m_structure[t0], t0
    313313    bbneq Structure::m_typeInfo + TypeInfo::m_type[t0], FinalObjectType, .opToThisSlow
    314     valueProfile(t0, 2, t1)
     314    loadpFromInstruction(2, t2)
     315    bpneq t0, t2, .opToThisSlow
    315316    dispatch(3)
    316317

  • trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

    r156376 r156468  
    231231    BEGIN();
    232232    JSValue v1 = OP(1).jsValue();
    233 #if ENABLE(VALUE_PROFILER)
    234     pc[OPCODE_LENGTH(op_to_this) - 1].u.profile->m_buckets[0] =
    235         JSValue::encode(v1.structureOrUndefined());
    236 #endif
     233    if (v1.isCell())
     234        pc[2].u.structure.set(exec->vm(), exec->codeBlock()->ownerExecutable(), v1.asCell()->structure());
     235    else
     236        pc[2].u.structure.clear();
    237237    RETURN(v1.toThis(exec, exec->codeBlock()->isStrictMode() ? StrictMode : NotStrictMode));
    238238}
Note: See TracChangeset for help on using the changeset viewer.