Changeset 134151 in webkit


Ignore:
Timestamp:
Nov 9, 2012, 9:54:11 PM (12 years ago)
Author:
fpizlo@apple.com
Message:

If the DFG ArrayMode says that an access is on an OriginalArray, then the checks should always enforce this
​https://bugs.webkit.org/show_bug.cgi?id=101720

Reviewed by Mark Hahnenberg.

Previously, "original" arrays was just a hint that we could find the structure
of the array if we needed to even if the array profile didn't have it due to
polymorphism. Now, "original" arrays are a property that is actually checked:
if an array access has ArrayMode::arrayClass() == Array::OriginalArray, then we
can be sure that the code performing the access is dealing with not just a
JSArray, but a JSArray that has no named properties, no indexed accessors, and
the ArrayPrototype as its prototype. This will be useful for optimizations that
are being done as part of ​https://bugs.webkit.org/show_bug.cgi?id=101720.

  • dfg/DFGAbstractState.cpp:

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

  • dfg/DFGArrayMode.cpp:

(JSC::DFG::ArrayMode::originalArrayStructure):
(DFG):
(JSC::DFG::ArrayMode::alreadyChecked):

  • dfg/DFGArrayMode.h:

(JSC):
(DFG):
(JSC::DFG::ArrayMode::withProfile):
(ArrayMode):
(JSC::DFG::ArrayMode::benefitsFromOriginalArray):

  • dfg/DFGConstantFoldingPhase.cpp:

(JSC::DFG::ConstantFoldingPhase::foldConstants):

  • dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::checkArray):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::jumpSlowForUnwantedArrayMode):
(JSC::DFG::SpeculativeJIT::checkArray):
(JSC::DFG::SpeculativeJIT::compileGetByValOnString):
(JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):
(JSC::DFG::SpeculativeJIT::compileGetByValOnFloatTypedArray):
(JSC::DFG::SpeculativeJIT::compilePutByValForFloatTypedArray):
(JSC::DFG::SpeculativeJIT::compileGetByValOnArguments):
(JSC::DFG::SpeculativeJIT::compileGetArgumentsLength):

Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r134134 r134151  
     12012-11-09  Filip Pizlo  <fpizlo@apple.com>
     2
     3        If the DFG ArrayMode says that an access is on an OriginalArray, then the checks should always enforce this
     4        https://bugs.webkit.org/show_bug.cgi?id=101720
     5
     6        Reviewed by Mark Hahnenberg.
     7
     8        Previously, "original" arrays was just a hint that we could find the structure
     9        of the array if we needed to even if the array profile didn't have it due to
     10        polymorphism. Now, "original" arrays are a property that is actually checked:
     11        if an array access has ArrayMode::arrayClass() == Array::OriginalArray, then we
     12        can be sure that the code performing the access is dealing with not just a
     13        JSArray, but a JSArray that has no named properties, no indexed accessors, and
     14        the ArrayPrototype as its prototype. This will be useful for optimizations that
     15        are being done as part of https://bugs.webkit.org/show_bug.cgi?id=101720.
     16
     17        * dfg/DFGAbstractState.cpp:
     18        (JSC::DFG::AbstractState::execute):
     19        * dfg/DFGArrayMode.cpp:
     20        (JSC::DFG::ArrayMode::originalArrayStructure):
     21        (DFG):
     22        (JSC::DFG::ArrayMode::alreadyChecked):
     23        * dfg/DFGArrayMode.h:
     24        (JSC):
     25        (DFG):
     26        (JSC::DFG::ArrayMode::withProfile):
     27        (ArrayMode):
     28        (JSC::DFG::ArrayMode::benefitsFromOriginalArray):
     29        * dfg/DFGConstantFoldingPhase.cpp:
     30        (JSC::DFG::ConstantFoldingPhase::foldConstants):
     31        * dfg/DFGFixupPhase.cpp:
     32        (JSC::DFG::FixupPhase::checkArray):
     33        * dfg/DFGSpeculativeJIT.cpp:
     34        (JSC::DFG::SpeculativeJIT::jumpSlowForUnwantedArrayMode):
     35        (JSC::DFG::SpeculativeJIT::checkArray):
     36        (JSC::DFG::SpeculativeJIT::compileGetByValOnString):
     37        (JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):
     38        (JSC::DFG::SpeculativeJIT::compileGetByValOnFloatTypedArray):
     39        (JSC::DFG::SpeculativeJIT::compilePutByValForFloatTypedArray):
     40        (JSC::DFG::SpeculativeJIT::compileGetByValOnArguments):
     41        (JSC::DFG::SpeculativeJIT::compileGetArgumentsLength):
     42
    1432012-11-09  Filip Pizlo  <fpizlo@apple.com>
    244

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

    r133990 r134151  
    14531453        break;
    14541454    case CheckArray: {
    1455         if (node.arrayMode().alreadyChecked(forNode(node.child1()))) {
     1455        if (node.arrayMode().alreadyChecked(m_graph, node, forNode(node.child1()))) {
    14561456            m_foundConstants = true;
    14571457            node.setCanExit(false);
    … …  
    15091509    }
    15101510    case Arrayify: {
    1511         if (node.arrayMode().alreadyChecked(forNode(node.child1()))) {
     1511        if (node.arrayMode().alreadyChecked(m_graph, node, forNode(node.child1()))) {
    15121512            m_foundConstants = true;
    15131513            node.setCanExit(false);

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

    r133953 r134151  
    3030
    3131#include "DFGAbstractValue.h"
     32#include "DFGGraph.h"
    3233
    3334namespace JSC { namespace DFG {
    … …  
    201202}
    202203
    203 bool ArrayMode::alreadyChecked(AbstractValue& value, IndexingType shape) const
    204 {
    205     if (isJSArray()) {
     204Structure* ArrayMode::originalArrayStructure(Graph& graph, const CodeOrigin& codeOrigin) const
     205{
     206    if (!isJSArrayWithOriginalStructure())
     207        return 0;
     208   
     209    JSGlobalObject* globalObject = graph.globalObjectFor(codeOrigin);
     210   
     211    switch (type()) {
     212    case Array::Int32:
     213        return globalObject->originalArrayStructureForIndexingType(ArrayWithInt32);
     214    case Array::Double:
     215        return globalObject->originalArrayStructureForIndexingType(ArrayWithDouble);
     216    case Array::Contiguous:
     217        return globalObject->originalArrayStructureForIndexingType(ArrayWithContiguous);
     218    case Array::ArrayStorage:
     219        return globalObject->originalArrayStructureForIndexingType(ArrayWithArrayStorage);
     220    default:
     221        CRASH();
     222        return 0;
     223    }
     224}
     225
     226Structure* ArrayMode::originalArrayStructure(Graph& graph, Node& node) const
     227{
     228    return originalArrayStructure(graph, node.codeOrigin);
     229}
     230
     231bool ArrayMode::alreadyChecked(Graph& graph, Node& node, AbstractValue& value, IndexingType shape) const
     232{
     233    switch (arrayClass()) {
     234    case Array::OriginalArray:
     235        return value.m_currentKnownStructure.hasSingleton()
     236            && (value.m_currentKnownStructure.singleton()->indexingType() & IndexingShapeMask) == shape
     237            && (value.m_currentKnownStructure.singleton()->indexingType() & IsArray)
     238            && graph.globalObjectFor(node.codeOrigin)->isOriginalArrayStructure(value.m_currentKnownStructure.singleton());
     239       
     240    case Array::Array:
    206241        if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModes(shape | IsArray)))
    207242            return true;
    … …  
    209244            && (value.m_currentKnownStructure.singleton()->indexingType() & IndexingShapeMask) == shape
    210245            && (value.m_currentKnownStructure.singleton()->indexingType() & IsArray);
    211     }
    212     if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModes(shape) | asArrayModes(shape | IsArray)))
    213         return true;
    214     return value.m_currentKnownStructure.hasSingleton()
    215         && (value.m_currentKnownStructure.singleton()->indexingType() & IndexingShapeMask) == shape;
    216 }
    217 
    218 bool ArrayMode::alreadyChecked(AbstractValue& value) const
     246       
     247    default:
     248        if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModes(shape) | asArrayModes(shape | IsArray)))
     249            return true;
     250        return value.m_currentKnownStructure.hasSingleton()
     251            && (value.m_currentKnownStructure.singleton()->indexingType() & IndexingShapeMask) == shape;
     252    }
     253}
     254
     255bool ArrayMode::alreadyChecked(Graph& graph, Node& node, AbstractValue& value) const
    219256{
    220257    switch (type()) {
    … …  
    229266       
    230267    case Array::Int32:
    231         return alreadyChecked(value, Int32Shape);
     268        return alreadyChecked(graph, node, value, Int32Shape);
    232269       
    233270    case Array::Double:
    234         return alreadyChecked(value, DoubleShape);
     271        return alreadyChecked(graph, node, value, DoubleShape);
    235272       
    236273    case Array::Contiguous:
    237         return alreadyChecked(value, ContiguousShape);
     274        return alreadyChecked(graph, node, value, ContiguousShape);
    238275       
    239276    case Array::ArrayStorage:
    240         return alreadyChecked(value, ArrayStorageShape);
     277        return alreadyChecked(graph, node, value, ArrayStorageShape);
    241278       
    242279    case Array::SlowPutArrayStorage:
    243         if (isJSArray()) {
     280        switch (arrayClass()) {
     281        case Array::OriginalArray:
     282            CRASH();
     283            return false;
     284       
     285        case Array::Array:
    244286            if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModes(ArrayWithArrayStorage) | asArrayModes(ArrayWithSlowPutArrayStorage)))
    245287                return true;
    … …  
    247289                && hasArrayStorage(value.m_currentKnownStructure.singleton()->indexingType())
    248290                && (value.m_currentKnownStructure.singleton()->indexingType() & IsArray);
     291       
     292        default:
     293            if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModes(NonArrayWithArrayStorage) | asArrayModes(ArrayWithArrayStorage) | asArrayModes(NonArrayWithSlowPutArrayStorage) | asArrayModes(ArrayWithSlowPutArrayStorage)))
     294                return true;
     295            return value.m_currentKnownStructure.hasSingleton()
     296                && hasArrayStorage(value.m_currentKnownStructure.singleton()->indexingType());
    249297        }
    250         if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModes(NonArrayWithArrayStorage) | asArrayModes(ArrayWithArrayStorage) | asArrayModes(NonArrayWithSlowPutArrayStorage) | asArrayModes(ArrayWithSlowPutArrayStorage)))
    251             return true;
    252         return value.m_currentKnownStructure.hasSingleton()
    253             && hasArrayStorage(value.m_currentKnownStructure.singleton()->indexingType());
    254298       
    255299    case Array::Arguments:

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

    r133953 r134151  
    3434#include "SpeculatedType.h"
    3535
    36 namespace JSC { namespace DFG {
    37 
     36namespace JSC {
     37
     38struct CodeOrigin;
     39
     40namespace DFG {
     41
     42class Graph;
    3843struct AbstractValue;
     44struct Node;
    3945
    4046// Use a namespace + enum instead of enum alone to avoid the namespace collision
    … …  
    162168       
    163169        if (isJSArray()) {
    164             if (profile->usesOriginalArrayStructures())
     170            if (profile->usesOriginalArrayStructures() && benefitsFromOriginalArray())
    165171                myArrayClass = Array::OriginalArray;
    166172            else
    … …  
    184190    ArrayMode refine(SpeculatedType base, SpeculatedType index, SpeculatedType value = SpecNone) const;
    185191   
    186     bool alreadyChecked(AbstractValue&) const;
     192    bool alreadyChecked(Graph&, Node&, AbstractValue&) const;
    187193   
    188194    const char* toString() const;
    … …  
    303309        }
    304310    }
     311   
     312    bool benefitsFromOriginalArray() const
     313    {
     314        switch (type()) {
     315        case Array::Int32:
     316        case Array::Double:
     317        case Array::Contiguous:
     318        case Array::ArrayStorage:
     319            return true;
     320        default:
     321            return false;
     322        }
     323    }
     324   
     325    // Returns 0 if this is not OriginalArray.
     326    Structure* originalArrayStructure(Graph&, const CodeOrigin&) const;
     327    Structure* originalArrayStructure(Graph&, Node&) const;
    305328   
    306329    bool benefitsFromStructureCheck() const
    … …  
    376399    }
    377400   
    378     bool alreadyChecked(AbstractValue&, IndexingType shape) const;
     401    bool alreadyChecked(Graph&, Node&, AbstractValue&, IndexingType shape) const;
    379402   
    380403    union {

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

    r132759 r134151  
    117117            case CheckArray:
    118118            case Arrayify: {
    119                 if (!node.arrayMode().alreadyChecked(m_state.forNode(node.child1())))
     119                if (!node.arrayMode().alreadyChecked(m_graph, node, m_state.forNode(node.child1())))
    120120                    break;
    121121                ASSERT(node.refCount() == 1);

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

    r133956 r134151  
    425425        m_graph.ref(array);
    426426
     427        Structure* structure = arrayMode.originalArrayStructure(m_graph, codeOrigin);
     428       
    427429        if (arrayMode.doesConversion()) {
    428430            if (index != NoNode)
    429431                m_graph.ref(index);
    430            
    431             Structure* structure = 0;
    432             if (arrayMode.isJSArrayWithOriginalStructure()) {
    433                 JSGlobalObject* globalObject = m_graph.baselineCodeBlockFor(codeOrigin)->globalObject();
    434                 switch (arrayMode.type()) {
    435                 case Array::Int32:
    436                     structure = globalObject->originalArrayStructureForIndexingType(ArrayWithInt32);
    437                     break;
    438                 case Array::Double:
    439                     structure = globalObject->originalArrayStructureForIndexingType(ArrayWithDouble);
    440                     break;
    441                 case Array::Contiguous:
    442                     structure = globalObject->originalArrayStructureForIndexingType(ArrayWithContiguous);
    443                     break;
    444                 case Array::ArrayStorage:
    445                     structure = globalObject->originalArrayStructureForIndexingType(ArrayWithArrayStorage);
    446                     break;
    447                 default:
    448                     break;
    449                 }
    450             }
    451432           
    452433            if (structure) {
    … …  
    464445            }
    465446        } else {
    466             Node checkArray(CheckArray, codeOrigin, OpInfo(arrayMode.asWord()), array);
    467             checkArray.ref();
    468             NodeIndex checkArrayIndex = m_graph.size();
    469             m_graph.append(checkArray);
    470             m_insertionSet.append(m_indexInBlock, checkArrayIndex);
     447            if (structure) {
     448                Node checkStructure(CheckStructure, codeOrigin, OpInfo(m_graph.addStructureSet(structure)), array);
     449                checkStructure.ref();
     450                NodeIndex checkStructureIndex = m_graph.size();
     451                m_graph.append(checkStructure);
     452                m_insertionSet.append(m_indexInBlock, checkStructureIndex);
     453            } else {
     454                Node checkArray(CheckArray, codeOrigin, OpInfo(arrayMode.asWord()), array);
     455                checkArray.ref();
     456                NodeIndex checkArrayIndex = m_graph.size();
     457                m_graph.append(checkArray);
     458                m_insertionSet.append(m_indexInBlock, checkArrayIndex);
     459            }
    471460        }
    472461       

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

    r133956 r134151  
    362362JITCompiler::Jump SpeculativeJIT::jumpSlowForUnwantedArrayMode(GPRReg tempGPR, ArrayMode arrayMode, IndexingType shape, bool invert)
    363363{
    364     if (arrayMode.isJSArray()) {
     364    switch (arrayMode.arrayClass()) {
     365    case Array::OriginalArray: {
     366        CRASH();
     367        JITCompiler::Jump result; // I already know that VC++ takes unkindly to the expression "return Jump()", so I'm doing it this way in anticipation of someone eventually using VC++ to compile the DFG.
     368        return result;
     369    }
     370       
     371    case Array::Array:
    365372        m_jit.and32(TrustedImm32(IsArray | IndexingShapeMask), tempGPR);
    366373        return m_jit.branch32(
    367374            invert ? MacroAssembler::Equal : MacroAssembler::NotEqual, tempGPR, TrustedImm32(IsArray | shape));
    368     }
    369     m_jit.and32(TrustedImm32(IndexingShapeMask), tempGPR);
    370     return m_jit.branch32(invert ? MacroAssembler::Equal : MacroAssembler::NotEqual, tempGPR, TrustedImm32(shape));
     375       
     376    default:
     377        m_jit.and32(TrustedImm32(IndexingShapeMask), tempGPR);
     378        return m_jit.branch32(invert ? MacroAssembler::Equal : MacroAssembler::NotEqual, tempGPR, TrustedImm32(shape));
     379    }
    371380}
    372381
    … …  
    387396    case Array::ArrayStorage:
    388397    case Array::SlowPutArrayStorage: {
     398        ASSERT(!arrayMode.isJSArrayWithOriginalStructure());
     399       
    389400        if (arrayMode.isJSArray()) {
    390401            if (arrayMode.isSlowPut()) {
    391402                if (invert) {
    392                     JITCompiler::Jump slow =
    393                         m_jit.branchTest32(
    394                             MacroAssembler::Zero, tempGPR, MacroAssembler::TrustedImm32(IsArray));
     403                    JITCompiler::Jump slow = m_jit.branchTest32(
     404                        MacroAssembler::Zero, tempGPR, MacroAssembler::TrustedImm32(IsArray));
    395405                    m_jit.and32(TrustedImm32(IndexingShapeMask), tempGPR);
    396406                    m_jit.sub32(TrustedImm32(ArrayStorageShape), tempGPR);
    … …  
    450460    const TypedArrayDescriptor* result = typedArrayDescriptor(node.arrayMode());
    451461   
    452     if (node.arrayMode().alreadyChecked(m_state.forNode(node.child1()))) {
     462    if (node.arrayMode().alreadyChecked(m_jit.graph(), node, m_state.forNode(node.child1()))) {
    453463        noResult(m_compileIndex);
    454464        return;
    … …  
    19601970    GPRReg storageReg = storage.gpr();
    19611971
    1962     ASSERT(ArrayMode(Array::String).alreadyChecked(m_state.forNode(node.child1())));
     1972    ASSERT(ArrayMode(Array::String).alreadyChecked(m_jit.graph(), node, m_state.forNode(node.child1())));
    19631973
    19641974    // unsigned comparison so we can filter out negative indices and indices that are too large
    … …  
    23692379    GPRReg resultReg = result.gpr();
    23702380
    2371     ASSERT(node.arrayMode().alreadyChecked(m_state.forNode(node.child1())));
     2381    ASSERT(node.arrayMode().alreadyChecked(m_jit.graph(), node, m_state.forNode(node.child1())));
    23722382
    23732383    speculationCheck(
    … …  
    25192529    GPRReg storageReg = storage.gpr();
    25202530
    2521     ASSERT(node.arrayMode().alreadyChecked(m_state.forNode(node.child1())));
     2531    ASSERT(node.arrayMode().alreadyChecked(m_jit.graph(), node, m_state.forNode(node.child1())));
    25222532
    25232533    FPRTemporary result(this);
    … …  
    25562566    SpeculateDoubleOperand valueOp(this, valueUse);
    25572567   
    2558     ASSERT_UNUSED(baseUse, node.arrayMode().alreadyChecked(m_state.forNode(baseUse)));
     2568    ASSERT_UNUSED(baseUse, node.arrayMode().alreadyChecked(m_jit.graph(), m_jit.graph()[m_compileIndex], m_state.forNode(baseUse)));
    25592569   
    25602570    GPRTemporary result(this);
    … …  
    33793389        return;
    33803390 
    3381     ASSERT(ArrayMode(Array::Arguments).alreadyChecked(m_state.forNode(node.child1())));
     3391    ASSERT(ArrayMode(Array::Arguments).alreadyChecked(m_jit.graph(), node, m_state.forNode(node.child1())));
    33823392   
    33833393    // Two really lame checks.
    … …  
    34363446        return;
    34373447   
    3438     ASSERT(ArrayMode(Array::Arguments).alreadyChecked(m_state.forNode(node.child1())));
     3448    ASSERT(ArrayMode(Array::Arguments).alreadyChecked(m_jit.graph(), node, m_state.forNode(node.child1())));
    34393449   
    34403450    speculationCheck(
Note: See TracChangeset for help on using the changeset viewer.