Changeset 258901 in webkit


Ignore:
Timestamp:
Mar 23, 2020 8:02:28 PM (4 years ago)
Author:
keith_miller@apple.com
Message:

HasIndexedProperty should know about sane chain
​https://bugs.webkit.org/show_bug.cgi?id=209457

Reviewed by Saam Barati.

This patch makes it so HasIndexedProperty is aware of
sane chain. This is useful because, most of the time we do an
indexed in it is on an array. If the array has a sane chain (i.e.
no indexed properties on it's prototypes and has the default
prototype chain) then we can just test for the index being a hole.

Note, we could also just convert OOB indices into false but that
should happen in another patch.
​https://bugs.webkit.org/show_bug.cgi?id=209456

I didn't add any tests because it turns out we already have a ton.
I know this because I broke most of them repeatedly... >.>

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

  • dfg/DFGClobberize.h:

(JSC::DFG::clobberize):

  • dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::setSaneChainIfPossible):
(JSC::DFG::FixupPhase::convertToHasIndexedProperty):

  • dfg/DFGNodeType.h:
  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileHasIndexedProperty):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
(JSC::FTL::DFG::LowerDFGToB3::speculateAndJump):

  • jit/AssemblyHelpers.h:

(JSC::AssemblyHelpers::isEmpty):

Location:
trunk/Source/JavaScriptCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r258887 r258901  
     12020-03-23  Keith Miller  <keith_miller@apple.com>
     2
     3        HasIndexedProperty should know about sane chain
     4        https://bugs.webkit.org/show_bug.cgi?id=209457
     5
     6        Reviewed by Saam Barati.
     7
     8        This patch makes it so HasIndexedProperty is aware of
     9        sane chain. This is useful because, most of the time we do an
     10        indexed in it is on an array. If the array has a sane chain (i.e.
     11        no indexed properties on it's prototypes and has the default
     12        prototype chain) then we can just test for the index being a hole.
     13
     14        Note, we could also just convert OOB indices into false but that
     15        should happen in another patch.
     16        https://bugs.webkit.org/show_bug.cgi?id=209456
     17
     18        I didn't add any tests because it turns out we already have a ton.
     19        I know this because I broke most of them repeatedly... >.>
     20
     21        * dfg/DFGAbstractInterpreterInlines.h:
     22        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
     23        * dfg/DFGClobberize.h:
     24        (JSC::DFG::clobberize):
     25        * dfg/DFGFixupPhase.cpp:
     26        (JSC::DFG::FixupPhase::fixupNode):
     27        (JSC::DFG::FixupPhase::setSaneChainIfPossible):
     28        (JSC::DFG::FixupPhase::convertToHasIndexedProperty):
     29        * dfg/DFGNodeType.h:
     30        * dfg/DFGSpeculativeJIT.cpp:
     31        (JSC::DFG::SpeculativeJIT::compileHasIndexedProperty):
     32        * ftl/FTLLowerDFGToB3.cpp:
     33        (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
     34        (JSC::FTL::DFG::LowerDFGToB3::speculateAndJump):
     35        * jit/AssemblyHelpers.h:
     36        (JSC::AssemblyHelpers::isEmpty):
     37
    1382020-03-23  Yusuke Suzuki  <ysuzuki@apple.com>
    239

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

    r254959 r258901  
    39193919        case Array::Contiguous:
    39203920        case Array::ArrayStorage: {
    3921             break;
     3921            if (mode.isInBounds())
     3922                break;
     3923            FALLTHROUGH;
    39223924        }
    39233925        default: {

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

    r258452 r258901  
    325325                return;
    326326            }
    327             read(Heap);
    328             return;
     327            break;
    329328        }
    330329           
    … …  
    336335                return;
    337336            }
    338             read(Heap);
    339             return;
     337            break;
    340338        }
    341339           
    … …  
    347345                return;
    348346            }
    349             read(Heap);
    350             return;
     347            break;
    351348        }
    352349
    … …  
    357354                return;
    358355            }
    359             read(Heap);
    360             return;
    361         }
    362 
    363         default: {
    364             read(World);
    365             write(Heap);
    366             return;
    367         }
    368         }
    369         RELEASE_ASSERT_NOT_REACHED();
     356            break;
     357        }
     358
     359        default:
     360            break;
     361        }
     362        read(World);
     363        write(Heap);
    370364        return;
    371365    }

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

    r258887 r258901  
    958958                    }
    959959                   
    960                     if (canDoSaneChain) {
    961                         JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
    962                         Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure(vm());
    963                         Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure(vm());
    964                         if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
    965                             && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
    966                             && globalObject->arrayPrototypeChainIsSane()) {
    967                             m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
    968                             m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
    969                             node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
    970                         }
    971                     }
     960                    if (canDoSaneChain)
     961                        setSaneChainIfPossible(node);
    972962                }
    973963                break;
    … …  
    20302020            fixEdge<CellUse>(m_graph.varArgChild(node, 0));
    20312021            fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
     2022
     2023            ArrayMode arrayMode = node->arrayMode();
     2024            // FIXME: OutOfBounds shouldn't preclude going sane chain because OOB is just false and cannot have effects.
     2025            // See: https://bugs.webkit.org/show_bug.cgi?id=209456
     2026            if (arrayMode.isJSArrayWithOriginalStructure() && arrayMode.speculation() == Array::InBounds)
     2027                setSaneChainIfPossible(node);
    20322028            break;
    20332029        }
    … …  
    33433339            OpInfo(arrayMode.asWord()), Edge(array, KnownCellUse));
    33443340    }
     3341
     3342    void setSaneChainIfPossible(Node* node)
     3343    {
     3344        JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
     3345        Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure(vm());
     3346        Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure(vm());
     3347        if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
     3348            && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
     3349            && globalObject->arrayPrototypeChainIsSane()) {
     3350            m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
     3351            m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
     3352            node->setArrayMode(node->arrayMode().withSpeculation(Array::SaneChain));
     3353            node->clearFlags(NodeMustGenerate);
     3354        }
     3355    }
    33453356   
    33463357    void blessArrayOperation(Edge base, Edge index, Edge& storageChild)
    … …  
    36953706    {
    36963707        node->setOp(HasIndexedProperty);
    3697         node->clearFlags(NodeMustGenerate);
    36983708
    36993709        {
    … …  
    37033713            m_graph.m_varArgChildren.append(node->child2());
    37043714            m_graph.m_varArgChildren.append(Edge());
    3705             node->mergeFlags(NodeHasVarArgs);
     3715            node->setFlags(defaultFlags(HasIndexedProperty));
    37063716            node->children = AdjacencyList(AdjacencyList::Variable, firstChild, numChildren);
    37073717        }
    … …  
    37163726
    37173727        blessArrayOperation(m_graph.varArgChild(node, 0), m_graph.varArgChild(node, 1), m_graph.varArgChild(node, 2));
     3728        auto arrayMode = node->arrayMode();
     3729        // FIXME: OutOfBounds shouldn't preclude going sane chain because OOB is just false and cannot have effects.
     3730        // See: https://bugs.webkit.org/show_bug.cgi?id=209456
     3731        if (arrayMode.isJSArrayWithOriginalStructure() && arrayMode.speculation() == Array::InBounds)
     3732            setSaneChainIfPossible(node);
    37183733
    37193734        fixEdge<CellUse>(m_graph.varArgChild(node, 0));

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

    r254936 r258901  
    484484    /* For-in enumeration opcodes */\
    485485    macro(GetEnumerableLength, NodeMustGenerate | NodeResultJS) \
    486     macro(HasIndexedProperty, NodeResultBoolean | NodeHasVarArgs) \
     486    /* Must generate because of Proxies on the prototype chain */ \
     487    macro(HasIndexedProperty, NodeMustGenerate | NodeResultBoolean | NodeHasVarArgs) \
    487488    macro(HasStructureProperty, NodeResultBoolean) \
    488489    macro(HasGenericProperty, NodeResultBoolean) \

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

    r258063 r258901  
    1358113581
    1358213582        MacroAssembler::Jump outOfBounds = m_jit.branch32(MacroAssembler::AboveOrEqual, indexGPR, MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()));
     13583
    1358313584        if (mode.isInBounds())
    1358413585            speculationCheck(OutOfBounds, JSValueRegs(), nullptr, outOfBounds);
    … …  
    1358813589#if USE(JSVALUE64)
    1358913590        m_jit.load64(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight), scratchGPR);
    13590         slowCases.append(m_jit.branchIfEmpty(scratchGPR));
    1359113591#else
    1359213592        m_jit.load32(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight, OBJECT_OFFSETOF(JSValue, u.asBits.tag)), scratchGPR);
    13593         slowCases.append(m_jit.branchIfEmpty(scratchGPR));
    1359413593#endif
     13594
     13595        if (mode.isSaneChain()) {
     13596            m_jit.isEmpty(scratchGPR, resultGPR);
     13597            break;
     13598        }
     13599
     13600        MacroAssembler::Jump isHole = m_jit.branchIfEmpty(scratchGPR);
     13601        if (!mode.isInBounds())
     13602            slowCases.append(isHole);
     13603        else
     13604            speculationCheck(LoadFromHole, JSValueRegs(), nullptr, isHole);
    1359513605        m_jit.move(TrustedImm32(1), resultGPR);
    1359613606        break;
    … …  
    1360413614
    1360513615        MacroAssembler::Jump outOfBounds = m_jit.branch32(MacroAssembler::AboveOrEqual, indexGPR, MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()));
     13616
    1360613617        if (mode.isInBounds())
    1360713618            speculationCheck(OutOfBounds, JSValueRegs(), nullptr, outOfBounds);
    … …  
    1361013621
    1361113622        m_jit.loadDouble(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight), scratchFPR);
    13612         slowCases.append(m_jit.branchIfNaN(scratchFPR));
     13623
     13624        if (mode.isSaneChain()) {
     13625            m_jit.compareDouble(MacroAssembler::DoubleEqualAndOrdered, scratchFPR, scratchFPR, resultGPR);
     13626            break;
     13627        }
     13628
     13629        MacroAssembler::Jump isHole = m_jit.branchIfNaN(scratchFPR);
     13630        if (!mode.isInBounds())
     13631            slowCases.append(isHole);
     13632        else
     13633            speculationCheck(LoadFromHole, JSValueRegs(), nullptr, isHole);
    1361313634        m_jit.move(TrustedImm32(1), resultGPR);
    1361413635        break;
    … …  
    1363013651#if USE(JSVALUE64)
    1363113652        m_jit.load64(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight, ArrayStorage::vectorOffset()), scratchGPR);
    13632         slowCases.append(m_jit.branchIfEmpty(scratchGPR));
    1363313653#else
    1363413654        m_jit.load32(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight, ArrayStorage::vectorOffset() + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), scratchGPR);
    13635         slowCases.append(m_jit.branchIfEmpty(scratchGPR));
    1363613655#endif
     13656
     13657        if (mode.isSaneChain()) {
     13658            m_jit.isEmpty(scratchGPR, resultGPR);
     13659            break;
     13660        }
     13661
     13662        MacroAssembler::Jump isHole = m_jit.branchIfEmpty(scratchGPR);
     13663        if (!mode.isInBounds() || mode.isSaneChain())
     13664            slowCases.append(isHole);
     13665        else
     13666            speculationCheck(LoadFromHole, JSValueRegs(), nullptr, isHole);
    1363713667        m_jit.move(TrustedImm32(1), resultGPR);
    1363813668        break;

  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r258887 r258901  
    1159411594        LValue base = lowCell(m_graph.varArgChild(m_node, 0));
    1159511595        LValue index = lowInt32(m_graph.varArgChild(m_node, 1));
     11596        ArrayMode mode = m_node->arrayMode();
    1159611597
    1159711598        switch (m_node->arrayMode().type()) {
    … …  
    1160111602            LValue internalMethodType = m_out.constInt32(static_cast<int32_t>(m_node->internalMethodType()));
    1160211603
    11603             IndexedAbstractHeap& heap = m_node->arrayMode().type() == Array::Int32 ?
     11604            IndexedAbstractHeap& heap = mode.type() == Array::Int32 ?
    1160411605                m_heaps.indexedInt32Properties : m_heaps.indexedContiguousProperties;
    1160511606
    … …  
    1160811609            LBasicBlock lastNext = nullptr;
    1160911610
     11611            if (!mode.isInBounds()) {
     11612                LBasicBlock checkHole = m_out.newBlock();
     11613                m_out.branch(
     11614                    m_out.aboveOrEqual(
     11615                        index, m_out.load32NonNegative(storage, m_heaps.Butterfly_publicLength)),
     11616                    rarely(slowCase), usually(checkHole));
     11617                lastNext = m_out.appendTo(checkHole, slowCase);
     11618            } else
     11619                lastNext = m_out.insertNewBlocksBefore(slowCase);
     11620
     11621            LValue checkHoleResultValue =
     11622                m_out.notZero64(m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1))));
     11623            ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
     11624            if (mode.isSaneChain())
     11625                m_out.jump(continuation);
     11626            else if (!mode.isInBounds())
     11627                m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
     11628            else
     11629                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
     11630
     11631            m_out.appendTo(slowCase, continuation);
     11632            ValueFromBlock slowResult = m_out.anchor(
     11633                m_out.notZero64(vmCall(Int64, operationHasIndexedPropertyByInt, weakPointer(globalObject), base, index, internalMethodType)));
     11634            m_out.jump(continuation);
     11635
     11636            m_out.appendTo(continuation, lastNext);
     11637            setBoolean(m_out.phi(Int32, checkHoleResult, slowResult));
     11638            return;
     11639        }
     11640        case Array::Double: {
     11641            LValue storage = lowStorage(m_graph.varArgChild(m_node, 2));
     11642            LValue internalMethodType = m_out.constInt32(static_cast<int32_t>(m_node->internalMethodType()));
     11643           
     11644            IndexedAbstractHeap& heap = m_heaps.indexedDoubleProperties;
     11645           
     11646            LBasicBlock slowCase = m_out.newBlock();
     11647            LBasicBlock continuation = m_out.newBlock();
     11648            LBasicBlock lastNext = nullptr;
     11649           
    1161011650            if (!m_node->arrayMode().isInBounds()) {
    1161111651                LBasicBlock checkHole = m_out.newBlock();
    … …  
    1161811658                lastNext = m_out.insertNewBlocksBefore(slowCase);
    1161911659
    11620             LValue checkHoleResultValue =
    11621                 m_out.notZero64(m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1))));
    11622             ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
    11623             m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
    11624 
    11625             m_out.appendTo(slowCase, continuation);
    11626             ValueFromBlock slowResult = m_out.anchor(
    11627                 m_out.notZero64(vmCall(Int64, operationHasIndexedPropertyByInt, weakPointer(globalObject), base, index, internalMethodType)));
    11628             m_out.jump(continuation);
    11629 
    11630             m_out.appendTo(continuation, lastNext);
    11631             setBoolean(m_out.phi(Int32, checkHoleResult, slowResult));
    11632             return;
    11633         }
    11634         case Array::Double: {
    11635             LValue storage = lowStorage(m_graph.varArgChild(m_node, 2));
    11636             LValue internalMethodType = m_out.constInt32(static_cast<int32_t>(m_node->internalMethodType()));
    11637            
    11638             IndexedAbstractHeap& heap = m_heaps.indexedDoubleProperties;
    11639            
    11640             LBasicBlock slowCase = m_out.newBlock();
    11641             LBasicBlock continuation = m_out.newBlock();
    11642             LBasicBlock lastNext = nullptr;
    11643            
    11644             if (!m_node->arrayMode().isInBounds()) {
    11645                 LBasicBlock checkHole = m_out.newBlock();
    11646                 m_out.branch(
    11647                     m_out.aboveOrEqual(
    11648                         index, m_out.load32NonNegative(storage, m_heaps.Butterfly_publicLength)),
    11649                     rarely(slowCase), usually(checkHole));
    11650                 lastNext = m_out.appendTo(checkHole, slowCase);
    11651             } else
    11652                 lastNext = m_out.insertNewBlocksBefore(slowCase);
    11653 
    1165411660            LValue doubleValue = m_out.loadDouble(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1)));
    1165511661            LValue checkHoleResultValue = m_out.doubleEqual(doubleValue, doubleValue);
    1165611662            ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
    11657             m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
     11663            if (mode.isSaneChain())
     11664                m_out.jump(continuation);
     11665            else if (!mode.isInBounds())
     11666                m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
     11667            else
     11668                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
    1165811669           
    1165911670            m_out.appendTo(slowCase, continuation);
    … …  
    1168811699                m_out.notZero64(m_out.load64(baseIndex(m_heaps.ArrayStorage_vector, storage, index, m_graph.varArgChild(m_node, 1))));
    1168911700            ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
    11690             m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
     11701            if (mode.isSaneChain())
     11702                m_out.jump(continuation);
     11703            else if (!mode.isInBounds())
     11704                m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
     11705            else
     11706                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
    1169111707
    1169211708            m_out.appendTo(slowCase, continuation);
    … …  
    1612716143        appendOSRExit(kind, lowValue, profile, failCondition, m_origin);
    1612816144    }
     16145
     16146    template<typename... Args>
     16147    void speculateAndJump(B3::BasicBlock* target, Args... args)
     16148    {
     16149        speculate(args...);
     16150        m_out.jump(target);
     16151    }
    1612916152   
    1613016153    void terminate(ExitKind kind)

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

    r258063 r258901  
    962962    Jump branchIfNotFunction(GPRReg cellGPR) { return branchIfNotType(cellGPR, JSFunctionType); }
    963963   
     964    void isEmpty(GPRReg gpr, GPRReg dst)
     965    {
     966#if USE(JSVALUE64)
     967        test64(NonZero, gpr, TrustedImm32(-1), dst);
     968#else
     969        compare32(Equal, gpr, TrustedImm32(JSValue::EmptyValueTag), dst);
     970#endif
     971    }
     972
    964973    Jump branchIfEmpty(GPRReg gpr)
    965974    {
Note: See TracChangeset for help on using the changeset viewer.