Changeset 245192 in webkit


Ignore:
Timestamp:
May 10, 2019 1:37:07 PM (5 years ago)
Author:
rmorisset@apple.com
Message:

testb3 failing with crash in JSC::B3::BasicBlock::appendNonTerminal
https://bugs.webkit.org/show_bug.cgi?id=197756
<rdar://problem/50641659>

Reviewed by Saam Barati.

When I added https://bugs.webkit.org/show_bug.cgi?id=197265 I assumed that which block is the root does not change in the middle of strength reduction.
But specializeSelect can use splitForward, which uses a new block for the first half of the given block.
So if the block being split is the root block I must update m_root and erase the m_valueInConstant cache.
Erasing the cache cannot cause wrong results: at most it can make us miss some optimization opportunities in this iteration of the fixpoint.

  • b3/B3ReduceStrength.cpp:
Location:
trunk/Source/JavaScriptCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r245168 r245192  
     12019-05-10  Robin Morisset  <rmorisset@apple.com>
     2
     3        testb3 failing with crash in JSC::B3::BasicBlock::appendNonTerminal
     4        https://bugs.webkit.org/show_bug.cgi?id=197756
     5        <rdar://problem/50641659>
     6
     7        Reviewed by Saam Barati.
     8
     9        When I added https://bugs.webkit.org/show_bug.cgi?id=197265 I assumed that which block is the root does not change in the middle of strength reduction.
     10        But specializeSelect can use splitForward, which uses a new block for the first half of the given block.
     11        So if the block being split is the root block I must update m_root and erase the m_valueInConstant cache.
     12        Erasing the cache cannot cause wrong results: at most it can make us miss some optimization opportunities in this iteration of the fixpoint.
     13
     14        * b3/B3ReduceStrength.cpp:
     15
    1162019-05-09  Keith Miller  <keith_miller@apple.com>
    217
  • trunk/Source/JavaScriptCore/b3/B3ReduceStrength.cpp

    r245035 r245192  
    21632163            else {
    21642164                Value* constInRoot = m_proc.clone(m_value);
     2165                ASSERT(m_root && m_root->size() >= 1);
    21652166                m_root->appendNonTerminal(constInRoot);
    21662167                m_valueForConstant.add(key, constInRoot);
     
    22252226        // This mutates startIndex to account for the fact that m_block got the front of it
    22262227        // chopped off.
    2227         BasicBlock* predecessor =
    2228             m_blockInsertionSet.splitForward(m_block, m_index, &m_insertionSet);
     2228        BasicBlock* predecessor = m_blockInsertionSet.splitForward(m_block, m_index, &m_insertionSet);
     2229        if (m_block == m_root) {
     2230            m_root = predecessor;
     2231            m_valueForConstant.clear();
     2232        }
    22292233
    22302234        // Splitting will commit the insertion set, which changes the exact position of the
Note: See TracChangeset for help on using the changeset viewer.