Changeset 124555 in webkit


Ignore:
Timestamp:
Aug 2, 2012 8:27:08 PM (12 years ago)
Author:
fpizlo@apple.com
Message:
ASSERTION FAILED: at(m_compileIndex).canExit()
m_isCheckingArgumentTypes

​https://bugs.webkit.org/show_bug.cgi?id=91074

Reviewed by Mark Hahnenberg.

Source/JavaScriptCore:

Fixes a bug where the speculative JIT was performing an unnecessary speculation that the
CFA had proven shouldn't be performed, leading to asserts that a node should not have
exit sites. This is a debug-only assert with no release symptom - we were just emitting
a check that was not reachable.

Also found, and fixed, a bug where structure check hoisting was slightly confusing the
CFA by inserting GetLocal's into the graph. CSE would clean the GetLocal's up, which
would make the backend happy - but the CFA would produce subtly wrong results.

  • bytecode/SpeculatedType.h:

(JSC::isOtherOrEmptySpeculation):
(JSC):

  • dfg/DFGDriver.cpp:

(JSC::DFG::compile):

  • dfg/DFGGraph.cpp:

(JSC::DFG::Graph::dump):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compileObjectToObjectOrOtherEquality):
(JSC::DFG::SpeculativeJIT::compilePeepHoleObjectToObjectOrOtherEquality):

LayoutTests:

Added a test for this specific case (dfg-peephole-compare-final-object-to-final-object-or-other-when-both-proven-final-object)
as well as three other tests to cover similar, though not necessarily currently broken, cases, since it was previously the
case that we apparently had no explicit coverage for these code paths.

  • fast/js/dfg-compare-final-object-to-final-object-or-other-when-both-proven-final-object-expected.txt: Added.
  • fast/js/dfg-compare-final-object-to-final-object-or-other-when-both-proven-final-object.html: Added.
  • fast/js/dfg-compare-final-object-to-final-object-or-other-when-proven-final-object-expected.txt: Added.
  • fast/js/dfg-compare-final-object-to-final-object-or-other-when-proven-final-object.html: Added.
  • fast/js/dfg-peephole-compare-final-object-to-final-object-or-other-when-both-proven-final-object-expected.txt: Added.
  • fast/js/dfg-peephole-compare-final-object-to-final-object-or-other-when-both-proven-final-object.html: Added.
  • fast/js/dfg-peephole-compare-final-object-to-final-object-or-other-when-proven-final-object-expected.txt: Added.
  • fast/js/dfg-peephole-compare-final-object-to-final-object-or-other-when-proven-final-object.html: Added.
  • fast/js/script-tests/dfg-compare-final-object-to-final-object-or-other-when-both-proven-final-object.js: Added.

(foo):

  • fast/js/script-tests/dfg-compare-final-object-to-final-object-or-other-when-proven-final-object.js: Added.

(foo):

  • fast/js/script-tests/dfg-peephole-compare-final-object-to-final-object-or-other-when-both-proven-final-object.js: Added.

(foo):

  • fast/js/script-tests/dfg-peephole-compare-final-object-to-final-object-or-other-when-proven-final-object.js: Added.

(foo):

Location:
trunk
Files:
12 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r124554 r124555  
     12012-08-02  Filip Pizlo  <fpizlo@apple.com>
     2
     3        ASSERTION FAILED: at(m_compileIndex).canExit() || m_isCheckingArgumentTypes
     4        https://bugs.webkit.org/show_bug.cgi?id=91074
     5
     6        Reviewed by Mark Hahnenberg.
     7
     8        Added a test for this specific case (dfg-peephole-compare-final-object-to-final-object-or-other-when-both-proven-final-object)
     9        as well as three other tests to cover similar, though not necessarily currently broken, cases, since it was previously the
     10        case that we apparently had no explicit coverage for these code paths.
     11
     12        * fast/js/dfg-compare-final-object-to-final-object-or-other-when-both-proven-final-object-expected.txt: Added.
     13        * fast/js/dfg-compare-final-object-to-final-object-or-other-when-both-proven-final-object.html: Added.
     14        * fast/js/dfg-compare-final-object-to-final-object-or-other-when-proven-final-object-expected.txt: Added.
     15        * fast/js/dfg-compare-final-object-to-final-object-or-other-when-proven-final-object.html: Added.
     16        * fast/js/dfg-peephole-compare-final-object-to-final-object-or-other-when-both-proven-final-object-expected.txt: Added.
     17        * fast/js/dfg-peephole-compare-final-object-to-final-object-or-other-when-both-proven-final-object.html: Added.
     18        * fast/js/dfg-peephole-compare-final-object-to-final-object-or-other-when-proven-final-object-expected.txt: Added.
     19        * fast/js/dfg-peephole-compare-final-object-to-final-object-or-other-when-proven-final-object.html: Added.
     20        * fast/js/script-tests/dfg-compare-final-object-to-final-object-or-other-when-both-proven-final-object.js: Added.
     21        (foo):
     22        * fast/js/script-tests/dfg-compare-final-object-to-final-object-or-other-when-proven-final-object.js: Added.
     23        (foo):
     24        * fast/js/script-tests/dfg-peephole-compare-final-object-to-final-object-or-other-when-both-proven-final-object.js: Added.
     25        (foo):
     26        * fast/js/script-tests/dfg-peephole-compare-final-object-to-final-object-or-other-when-proven-final-object.js: Added.
     27        (foo):
     28
    1292012-08-02  Yoshifumi Inoue  <yosin@chromium.org>
    230

  • trunk/Source/JavaScriptCore/ChangeLog

    r124497 r124555  
     12012-08-02  Filip Pizlo  <fpizlo@apple.com>
     2
     3        ASSERTION FAILED: at(m_compileIndex).canExit() || m_isCheckingArgumentTypes
     4        https://bugs.webkit.org/show_bug.cgi?id=91074
     5
     6        Reviewed by Mark Hahnenberg.
     7
     8        Fixes a bug where the speculative JIT was performing an unnecessary speculation that the
     9        CFA had proven shouldn't be performed, leading to asserts that a node should not have
     10        exit sites. This is a debug-only assert with no release symptom - we were just emitting
     11        a check that was not reachable.
     12       
     13        Also found, and fixed, a bug where structure check hoisting was slightly confusing the
     14        CFA by inserting GetLocal's into the graph. CSE would clean the GetLocal's up, which
     15        would make the backend happy - but the CFA would produce subtly wrong results.
     16
     17        * bytecode/SpeculatedType.h:
     18        (JSC::isOtherOrEmptySpeculation):
     19        (JSC):
     20        * dfg/DFGDriver.cpp:
     21        (JSC::DFG::compile):
     22        * dfg/DFGGraph.cpp:
     23        (JSC::DFG::Graph::dump):
     24        * dfg/DFGSpeculativeJIT64.cpp:
     25        (JSC::DFG::SpeculativeJIT::compileObjectToObjectOrOtherEquality):
     26        (JSC::DFG::SpeculativeJIT::compilePeepHoleObjectToObjectOrOtherEquality):
     27
    1282012-08-02  Filip Pizlo  <fpizlo@apple.com>
    229

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

    r119660 r124555  
    244244}
    245245
     246inline bool isOtherOrEmptySpeculation(SpeculatedType value)
     247{
     248    return !value || value == SpecOther;
     249}
     250
    246251inline bool isEmptySpeculation(SpeculatedType value)
    247252{

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

    r124404 r124555  
    103103        performFixup(dfg);
    104104    }
    105     if (performStructureCheckHoisting(dfg))
    106         performCFA(dfg); // Need to recompute CFA since nodes were added or changed.
     105    bool shouldRedoCFA = performStructureCheckHoisting(dfg);
    107106    performCSE(dfg, FixpointConverged);
     107    if (shouldRedoCFA)
     108        performCFA(dfg);
    108109#if DFG_ENABLE(DEBUG_VERBOSE)
    109110    dataLog("DFG optimization fixpoint converged in %u iterations.\n", cnt);

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

    r124404 r124555  
    208208    }
    209209
    210     if (node.flags()) {
     210    if (strlen(nodeFlagsAsString(node.flags()))) {
    211211        dataLog("%s%s", hasPrinted ? ", " : "", nodeFlagsAsString(node.flags()));
    212212        hasPrinted = true;

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

    r124476 r124555  
    15791579    // We know that within this branch, rightChild must not be a cell. Check if that is enough to
    15801580    // prove that it is either null or undefined.
    1581     if (!isOtherSpeculation(m_state.forNode(rightChild).m_type & ~SpecCell)) {
     1581    if (!isOtherOrEmptySpeculation(m_state.forNode(rightChild).m_type & ~SpecCell)) {
    15821582        m_jit.move(op2GPR, resultGPR);
    15831583        m_jit.andPtr(MacroAssembler::TrustedImm32(~TagBitUndefined), resultGPR);
    … …  
    16501650    // We know that within this branch, rightChild must not be a cell. Check if that is enough to
    16511651    // prove that it is either null or undefined.
    1652     if (isOtherSpeculation(m_state.forNode(rightChild).m_type & ~SpecCell))
     1652    if (isOtherOrEmptySpeculation(m_state.forNode(rightChild).m_type & ~SpecCell))
    16531653        rightNotCell.link(&m_jit);
    16541654    else {
Note: See TracChangeset for help on using the changeset viewer.