Changeset 155567 in webkit


Ignore:
Timestamp:
Sep 11, 2013 2:49:47 PM (11 years ago)
Author:
fpizlo@apple.com
Message:

Int32ToDouble should be predicted SpecInt48 and predictions should have nothing to do with constant folding
​https://bugs.webkit.org/show_bug.cgi?id=121141

Source/JavaScriptCore:

Reviewed by Oliver Hunt.

Just changing Int32ToDouble to be predicted SpecInt48 breaks constant folding on that
node because of soooper old code that prevented constant folding on mismatched
predictions. Kill that code.

  • dfg/DFGAbstractInterpreter.h:

(JSC::DFG::AbstractInterpreter::setConstant):

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::::executeEffects):

  • dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::injectInt32ToDoubleNode):

LayoutTests:

Reviewed by Oliver Hunt.

Fixing this means that this test no longer recompiles.

  • js/script-tests/dfg-constant-fold-misprediction.js:
Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r155554 r155567  
     12013-09-10  Filip Pizlo  <fpizlo@apple.com>
     2
     3        Int32ToDouble should be predicted SpecInt48 and predictions should have nothing to do with constant folding
     4        https://bugs.webkit.org/show_bug.cgi?id=121141
     5
     6        Reviewed by Oliver Hunt.
     7       
     8        Fixing this means that this test no longer recompiles.
     9
     10        * js/script-tests/dfg-constant-fold-misprediction.js:
     11
    1122013-09-11  Myles C. Maxfield  <mmaxfield@apple.com>
    213

  • trunk/LayoutTests/js/script-tests/dfg-constant-fold-misprediction.js

    r155201 r155567  
    3535
    3636// Call foo() enough times to make it optimize three times.
    37 for (var i = 0; i < 2; i = dfgIncrement({f:foo, i:i + 1, n:1, compiles:2}))
     37// NOTE: We no longer recompile this three times.
     38for (var i = 0; i < 2; i = dfgIncrement({f:foo, i:i + 1, n:1}))
    3839    shouldBe("foo(0.5)", "1000.50025");
    3940

  • trunk/Source/JavaScriptCore/ChangeLog

    r155564 r155567  
     12013-09-11  Filip Pizlo  <fpizlo@apple.com>
     2
     3        Int32ToDouble should be predicted SpecInt48 and predictions should have nothing to do with constant folding
     4        https://bugs.webkit.org/show_bug.cgi?id=121141
     5
     6        Reviewed by Oliver Hunt.
     7       
     8        Just changing Int32ToDouble to be predicted SpecInt48 breaks constant folding on that
     9        node because of soooper old code that prevented constant folding on mismatched
     10        predictions. Kill that code.
     11
     12        * dfg/DFGAbstractInterpreter.h:
     13        (JSC::DFG::AbstractInterpreter::setConstant):
     14        * dfg/DFGAbstractInterpreterInlines.h:
     15        (JSC::DFG::::executeEffects):
     16        * dfg/DFGFixupPhase.cpp:
     17        (JSC::DFG::FixupPhase::injectInt32ToDoubleNode):
     18
    1192013-09-10  Filip Pizlo  <fpizlo@apple.com>
    220

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

    r154290 r155567  
    160160    BooleanResult booleanResult(Node*, AbstractValue&);
    161161   
    162     bool trySetConstant(Node* node, JSValue value)
     162    void setConstant(Node* node, JSValue value)
    163163    {
    164         // Make sure we don't constant fold something that will produce values that contravene
    165         // predictions. If that happens then we know that the code will OSR exit, forcing
    166         // recompilation. But if we tried to constant fold then we'll have a very degenerate
    167         // IR: namely we'll have a JSConstant that contravenes its own prediction. There's a
    168         // lot of subtle code that assumes that
    169         // speculationFromValue(jsConstant) == jsConstant.prediction(). "Hardening" that code
    170         // is probably less sane than just pulling back on constant folding.
    171         SpeculatedType oldType = node->prediction();
    172         if (mergeSpeculations(speculationFromValue(value), oldType) != oldType)
    173             return false;
    174        
    175164        forNode(node).set(m_graph, value);
    176         return true;
     165        m_state.setFoundConstants(true);
    177166    }
    178167   

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

    r155497 r155567  
    224224            int32_t a = left.asInt32();
    225225            int32_t b = right.asInt32();
    226             bool constantWasSet;
    227226            switch (node->op()) {
    228227            case BitAnd:
    229                 constantWasSet = trySetConstant(node, JSValue(a & b));
     228                setConstant(node, JSValue(a & b));
    230229                break;
    231230            case BitOr:
    232                 constantWasSet = trySetConstant(node, JSValue(a | b));
     231                setConstant(node, JSValue(a | b));
    233232                break;
    234233            case BitXor:
    235                 constantWasSet = trySetConstant(node, JSValue(a ^ b));
     234                setConstant(node, JSValue(a ^ b));
    236235                break;
    237236            case BitRShift:
    238                 constantWasSet = trySetConstant(node, JSValue(a >> static_cast<uint32_t>(b)));
     237                setConstant(node, JSValue(a >> static_cast<uint32_t>(b)));
    239238                break;
    240239            case BitLShift:
    241                 constantWasSet = trySetConstant(node, JSValue(a << static_cast<uint32_t>(b)));
     240                setConstant(node, JSValue(a << static_cast<uint32_t>(b)));
    242241                break;
    243242            case BitURShift:
    244                 constantWasSet = trySetConstant(node, JSValue(static_cast<uint32_t>(a) >> static_cast<uint32_t>(b)));
     243                setConstant(node, JSValue(static_cast<uint32_t>(a) >> static_cast<uint32_t>(b)));
    245244                break;
    246245            default:
    247246                RELEASE_ASSERT_NOT_REACHED();
    248                 constantWasSet = false;
     247                break;
    249248            }
    250             if (constantWasSet) {
    251                 m_state.setFoundConstants(true);
    252                 break;
    253             }
     249            break;
    254250        }
    255251        forNode(node).setType(SpecInt32);
    … …  
    261257        if (child && child.isNumber()) {
    262258            ASSERT(child.isInt32());
    263             if (trySetConstant(node, JSValue(child.asUInt32()))) {
    264                 m_state.setFoundConstants(true);
    265                 break;
    266             }
     259            setConstant(node, JSValue(child.asUInt32()));
     260            break;
    267261        }
    268262        if (!node->canSpeculateInt32())
    … …  
    280274            double asDouble = child.asNumber();
    281275            int32_t asInt = JSC::toInt32(asDouble);
    282             if (bitwise_cast<int64_t>(static_cast<double>(asInt)) == bitwise_cast<int64_t>(asDouble)
    283                 && trySetConstant(node, JSValue(asInt))) {
    284                 m_state.setFoundConstants(true);
     276            if (bitwise_cast<int64_t>(static_cast<double>(asInt)) == bitwise_cast<int64_t>(asDouble)) {
     277                setConstant(node, JSValue(asInt));
    285278                break;
    286279            }
    … …  
    294287        JSValue child = forNode(node->child1()).value();
    295288        if (child && child.isNumber()) {
    296             bool constantWasSet;
    297289            if (child.isInt32())
    298                 constantWasSet = trySetConstant(node, child);
     290                setConstant(node, child);
    299291            else
    300                 constantWasSet = trySetConstant(node, JSValue(JSC::toInt32(child.asDouble())));
    301             if (constantWasSet) {
    302                 m_state.setFoundConstants(true);
    303                 break;
    304             }
     292                setConstant(node, JSValue(JSC::toInt32(child.asDouble())));
     293            break;
    305294        }
    306295       
    … …  
    311300    case Int32ToDouble: {
    312301        JSValue child = forNode(node->child1()).value();
    313         if (child && child.isNumber()
    314             && trySetConstant(node, JSValue(JSValue::EncodeAsDouble, child.asNumber()))) {
    315             m_state.setFoundConstants(true);
     302        if (child && child.isNumber()) {
     303            setConstant(node, JSValue(JSValue::EncodeAsDouble, child.asNumber()));
    316304            break;
    317305        }
    … …  
    327315        JSValue left = forNode(node->child1()).value();
    328316        JSValue right = forNode(node->child2()).value();
    329         if (left && right && left.isNumber() && right.isNumber()
    330             && trySetConstant(node, JSValue(left.asNumber() + right.asNumber()))) {
    331             m_state.setFoundConstants(true);
     317        if (left && right && left.isNumber() && right.isNumber()) {
     318            setConstant(node, JSValue(left.asNumber() + right.asNumber()));
    332319            break;
    333320        }
    … …  
    362349        JSValue left = forNode(node->child1()).value();
    363350        JSValue right = forNode(node->child2()).value();
    364         if (left && right && left.isNumber() && right.isNumber()
    365             && trySetConstant(node, JSValue(left.asNumber() - right.asNumber()))) {
    366             m_state.setFoundConstants(true);
     351        if (left && right && left.isNumber() && right.isNumber()) {
     352            setConstant(node, JSValue(left.asNumber() - right.asNumber()));
    367353            break;
    368354        }
    … …  
    385371    case ArithNegate: {
    386372        JSValue child = forNode(node->child1()).value();
    387         if (child && child.isNumber()
    388             && trySetConstant(node, JSValue(-child.asNumber()))) {
    389             m_state.setFoundConstants(true);
     373        if (child && child.isNumber()) {
     374            setConstant(node, JSValue(-child.asNumber()));
    390375            break;
    391376        }
    … …  
    409394        JSValue left = forNode(node->child1()).value();
    410395        JSValue right = forNode(node->child2()).value();
    411         if (left && right && left.isNumber() && right.isNumber()
    412             && trySetConstant(node, JSValue(left.asNumber() * right.asNumber()))) {
    413             m_state.setFoundConstants(true);
     396        if (left && right && left.isNumber() && right.isNumber()) {
     397            setConstant(node, JSValue(left.asNumber() * right.asNumber()));
    414398            break;
    415399        }
    … …  
    446430        JSValue left = forNode(node->child1()).value();
    447431        JSValue right = forNode(node->child2()).value();
    448         if (node->op() == ArithMod && right && right.isNumber() && right.asNumber() == 1
    449             && trySetConstant(node, JSValue(0))) {
    450             m_state.setFoundConstants(true);
     432        if (node->op() == ArithMod && right && right.isNumber() && right.asNumber() == 1) {
     433            setConstant(node, JSValue(0));
    451434            break;
    452435        }
    … …  
    454437            double a = left.asNumber();
    455438            double b = right.asNumber();
    456             bool constantWasSet;
    457439            switch (node->op()) {
    458440            case ArithDiv:
    459                 constantWasSet = trySetConstant(node, JSValue(a / b));
     441                setConstant(node, JSValue(a / b));
    460442                break;
    461443            case ArithMin:
    462                 constantWasSet = trySetConstant(node, JSValue(a < b ? a : (b <= a ? b : a + b)));
     444                setConstant(node, JSValue(a < b ? a : (b <= a ? b : a + b)));
    463445                break;
    464446            case ArithMax:
    465                 constantWasSet = trySetConstant(node, JSValue(a > b ? a : (b >= a ? b : a + b)));
     447                setConstant(node, JSValue(a > b ? a : (b >= a ? b : a + b)));
    466448                break;
    467449            case ArithMod:
    468                 constantWasSet = trySetConstant(node, JSValue(fmod(a, b)));
     450                setConstant(node, JSValue(fmod(a, b)));
    469451                break;
    470452            default:
    471453                RELEASE_ASSERT_NOT_REACHED();
    472                 constantWasSet = false;
    473454                break;
    474455            }
    475             if (constantWasSet) {
    476                 m_state.setFoundConstants(true);
    477                 break;
    478             }
     456            break;
    479457        }
    480458        switch (node->binaryUseKind()) {
    … …  
    495473    case ArithAbs: {
    496474        JSValue child = forNode(node->child1()).value();
    497         if (child && child.isNumber()
    498             && trySetConstant(node, JSValue(fabs(child.asNumber())))) {
    499             m_state.setFoundConstants(true);
     475        if (child && child.isNumber()) {
     476            setConstant(node, JSValue(fabs(child.asNumber())));
    500477            break;
    501478        }
    … …  
    517494    case ArithSqrt: {
    518495        JSValue child = forNode(node->child1()).value();
    519         if (child && child.isNumber()
    520             && trySetConstant(node, JSValue(sqrt(child.asNumber())))) {
    521             m_state.setFoundConstants(true);
     496        if (child && child.isNumber()) {
     497            setConstant(node, JSValue(sqrt(child.asNumber())));
    522498            break;
    523499        }
    … …  
    527503           
    528504    case LogicalNot: {
    529         bool didSetConstant = false;
    530505        switch (booleanResult(node, forNode(node->child1()))) {
    531506        case DefinitelyTrue:
    532             didSetConstant = trySetConstant(node, jsBoolean(false));
     507            setConstant(node, jsBoolean(false));
    533508            break;
    534509        case DefinitelyFalse:
    535             didSetConstant = trySetConstant(node, jsBoolean(true));
     510            setConstant(node, jsBoolean(true));
    536511            break;
    537512        default:
    538             break;
    539         }
    540         if (didSetConstant) {
    541             m_state.setFoundConstants(true);
    542             break;
    543         }
    544         switch (node->child1().useKind()) {
    545         case BooleanUse:
    546         case Int32Use:
    547         case NumberUse:
    548         case UntypedUse:
    549             break;
    550         case ObjectOrOtherUse:
    551             node->setCanExit(true);
    552             break;
    553         default:
    554             RELEASE_ASSERT_NOT_REACHED();
    555             break;
    556         }
    557         forNode(node).setType(SpecBoolean);
     513            switch (node->child1().useKind()) {
     514            case BooleanUse:
     515            case Int32Use:
     516            case NumberUse:
     517            case UntypedUse:
     518                break;
     519            case ObjectOrOtherUse:
     520                node->setCanExit(true);
     521                break;
     522            default:
     523                RELEASE_ASSERT_NOT_REACHED();
     524                break;
     525            }
     526            forNode(node).setType(SpecBoolean);
     527            break;
     528        }
    558529        break;
    559530    }
    … …  
    570541        JSValue child = forNode(node->child1()).value();
    571542        if (child) {
    572             bool constantWasSet;
     543            bool constantWasSet = true;
    573544            switch (node->op()) {
    574545            case IsUndefined:
    575                 constantWasSet = trySetConstant(node, jsBoolean(
     546                setConstant(node, jsBoolean(
    576547                    child.isCell()
    577548                    ? child.asCell()->structure()->masqueradesAsUndefined(m_codeBlock->globalObjectFor(node->codeOrigin))
    … …  
    579550                break;
    580551            case IsBoolean:
    581                 constantWasSet = trySetConstant(node, jsBoolean(child.isBoolean()));
     552                setConstant(node, jsBoolean(child.isBoolean()));
    582553                break;
    583554            case IsNumber:
    584                 constantWasSet = trySetConstant(node, jsBoolean(child.isNumber()));
     555                setConstant(node, jsBoolean(child.isNumber()));
    585556                break;
    586557            case IsString:
    587                 constantWasSet = trySetConstant(node, jsBoolean(isJSString(child)));
     558                setConstant(node, jsBoolean(isJSString(child)));
    588559                break;
    589560            case IsObject:
    590561                if (child.isNull() || !child.isObject()) {
    591                     constantWasSet = trySetConstant(node, jsBoolean(child.isNull()));
     562                    setConstant(node, jsBoolean(child.isNull()));
    592563                    break;
    593564                }
     565                constantWasSet = false;
     566                break;
    594567            default:
    595568                constantWasSet = false;
    596569                break;
    597570            }
    598             if (constantWasSet) {
    599                 m_state.setFoundConstants(true);
    600                 break;
    601             }
     571            if (constantWasSet)
     572                break;
    602573        }
    603574
    … …  
    612583        if (child) {
    613584            JSValue typeString = jsTypeStringForValue(*vm, m_codeBlock->globalObjectFor(node->codeOrigin), child);
    614             if (trySetConstant(node, typeString)) {
    615                 m_state.setFoundConstants(true);
    616                 break;
    617             }
    618         } else if (isNumberSpeculation(abstractChild.m_type)) {
    619             if (trySetConstant(node, vm->smallStrings.numberString())) {
    620                 filter(node->child1(), SpecNumber);
    621                 m_state.setFoundConstants(true);
    622                 break;
    623             }
    624         } else if (isStringSpeculation(abstractChild.m_type)) {
    625             if (trySetConstant(node, vm->smallStrings.stringString())) {
    626                 filter(node->child1(), SpecString);
    627                 m_state.setFoundConstants(true);
    628                 break;
    629             }
    630         } else if (isFinalObjectSpeculation(abstractChild.m_type) || isArraySpeculation(abstractChild.m_type) || isArgumentsSpeculation(abstractChild.m_type)) {
    631             if (trySetConstant(node, vm->smallStrings.objectString())) {
    632                 filter(node->child1(), SpecFinalObject | SpecArray | SpecArguments);
    633                 m_state.setFoundConstants(true);
    634                 break;
    635             }
    636         } else if (isFunctionSpeculation(abstractChild.m_type)) {
    637             if (trySetConstant(node, vm->smallStrings.functionString())) {
    638                 filter(node->child1(), SpecFunction);
    639                 m_state.setFoundConstants(true);
    640                 break;
    641             }
    642         } else if (isBooleanSpeculation(abstractChild.m_type)) {
    643             if (trySetConstant(node, vm->smallStrings.booleanString())) {
    644                 filter(node->child1(), SpecBoolean);
    645                 m_state.setFoundConstants(true);
    646                 break;
    647             }
     585            setConstant(node, typeString);
     586            break;
     587        }
     588       
     589        if (isNumberSpeculation(abstractChild.m_type)) {
     590            setConstant(node, vm->smallStrings.numberString());
     591            break;
     592        }
     593       
     594        if (isStringSpeculation(abstractChild.m_type)) {
     595            setConstant(node, vm->smallStrings.stringString());
     596            break;
     597        }
     598       
     599        if (isFinalObjectSpeculation(abstractChild.m_type) || isArraySpeculation(abstractChild.m_type) || isArgumentsSpeculation(abstractChild.m_type)) {
     600            setConstant(node, vm->smallStrings.objectString());
     601            break;
     602        }
     603       
     604        if (isFunctionSpeculation(abstractChild.m_type)) {
     605            setConstant(node, vm->smallStrings.functionString());
     606            break;
     607        }
     608       
     609        if (isBooleanSpeculation(abstractChild.m_type)) {
     610            setConstant(node, vm->smallStrings.booleanString());
     611            break;
    648612        }
    649613
    … …  
    669633    case CompareEq:
    670634    case CompareEqConstant: {
    671         bool constantWasSet = false;
    672 
    673635        JSValue leftConst = forNode(node->child1()).value();
    674636        JSValue rightConst = forNode(node->child2()).value();
    … …  
    679641                switch (node->op()) {
    680642                case CompareLess:
    681                     constantWasSet = trySetConstant(node, jsBoolean(a < b));
     643                    setConstant(node, jsBoolean(a < b));
    682644                    break;
    683645                case CompareLessEq:
    684                     constantWasSet = trySetConstant(node, jsBoolean(a <= b));
     646                    setConstant(node, jsBoolean(a <= b));
    685647                    break;
    686648                case CompareGreater:
    687                     constantWasSet = trySetConstant(node, jsBoolean(a > b));
     649                    setConstant(node, jsBoolean(a > b));
    688650                    break;
    689651                case CompareGreaterEq:
    690                     constantWasSet = trySetConstant(node, jsBoolean(a >= b));
     652                    setConstant(node, jsBoolean(a >= b));
    691653                    break;
    692654                case CompareEq:
    693                     constantWasSet = trySetConstant(node, jsBoolean(a == b));
     655                    setConstant(node, jsBoolean(a == b));
    694656                    break;
    695657                default:
    696658                    RELEASE_ASSERT_NOT_REACHED();
    697                     constantWasSet = false;
     659                    break;
     660                }
     661                break;
     662            }
     663           
     664            if (node->op() == CompareEq && leftConst.isString() && rightConst.isString()) {
     665                const StringImpl* a = asString(leftConst)->tryGetValueImpl();
     666                const StringImpl* b = asString(rightConst)->tryGetValueImpl();
     667                if (a && b) {
     668                    setConstant(node, jsBoolean(WTF::equal(a, b)));
    698669                    break;
    699670                }
    700671            }
    701            
    702             if (!constantWasSet && node->op() == CompareEq
    703                 && leftConst.isString() && rightConst.isString()) {
    704                 const StringImpl* a = asString(leftConst)->tryGetValueImpl();
    705                 const StringImpl* b = asString(rightConst)->tryGetValueImpl();
    706                 if (a && b)
    707                     constantWasSet = trySetConstant(node, jsBoolean(WTF::equal(a, b)));
    708             }
    709         }
    710        
    711         if (!constantWasSet && (node->op() == CompareEqConstant || node->op() == CompareEq)) {
     672        }
     673       
     674        if (node->op() == CompareEqConstant || node->op() == CompareEq) {
    712675            SpeculatedType leftType = forNode(node->child1()).m_type;
    713676            SpeculatedType rightType = forNode(node->child2()).m_type;
    714677            if ((isInt32Speculation(leftType) && isOtherSpeculation(rightType))
    715                 || (isOtherSpeculation(leftType) && isInt32Speculation(rightType)))
    716                 constantWasSet = trySetConstant(node, jsBoolean(false));
    717         }
    718        
    719         if (constantWasSet) {
    720             m_state.setFoundConstants(true);
    721             break;
     678                || (isOtherSpeculation(leftType) && isInt32Speculation(rightType))) {
     679                setConstant(node, jsBoolean(false));
     680                break;
     681            }
    722682        }
    723683       
    … …  
    741701        JSValue right = forNode(rightNode).value();
    742702        if (left && right) {
    743             if (left.isNumber() && right.isNumber()
    744                 && trySetConstant(node, jsBoolean(left.asNumber() == right.asNumber()))) {
    745                 m_state.setFoundConstants(true);
     703            if (left.isNumber() && right.isNumber()) {
     704                setConstant(node, jsBoolean(left.asNumber() == right.asNumber()));
    746705                break;
    747706            }
    … …  
    749708                const StringImpl* a = asString(left)->tryGetValueImpl();
    750709                const StringImpl* b = asString(right)->tryGetValueImpl();
    751                 if (a && b && trySetConstant(node, jsBoolean(WTF::equal(a, b)))) {
    752                     m_state.setFoundConstants(true);
     710                if (a && b) {
     711                    setConstant(node, jsBoolean(WTF::equal(a, b)));
    753712                    break;
    754713                }
    … …  
    963922    case ToPrimitive: {
    964923        JSValue childConst = forNode(node->child1()).value();
    965         if (childConst && childConst.isNumber() && trySetConstant(node, childConst)) {
    966             m_state.setFoundConstants(true);
     924        if (childConst && childConst.isNumber()) {
     925            setConstant(node, childConst);
    967926            break;
    968927        }
    … …  
    12081167    case SkipScope: {
    12091168        JSValue child = forNode(node->child1()).value();
    1210         if (child && trySetConstant(node, JSValue(jsCast<JSScope*>(child.asCell())->next()))) {
    1211             m_state.setFoundConstants(true);
     1169        if (child) {
     1170            setConstant(node, JSValue(jsCast<JSScope*>(child.asCell())->next()));
    12121171            break;
    12131172        }
    … …  
    12471206                   
    12481207                    if (status.specificValue())
    1249                         forNode(node).set(m_graph, status.specificValue());
     1208                        setConstant(node, status.specificValue());
    12501209                    else
    12511210                        forNode(node).makeHeapTop();

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

    r155564 r155567  
    13711371    {
    13721372        Node* result = m_insertionSet.insertNode(
    1373             m_indexInBlock, SpecDouble, Int32ToDouble,
     1373            m_indexInBlock, SpecInt48, Int32ToDouble,
    13741374            m_currentNode->codeOrigin, Edge(edge.node(), NumberUse));
    13751375        if (direction == ForwardSpeculation)
Note: See TracChangeset for help on using the changeset viewer.