⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 240376 in webkit


Ignore:
Timestamp:
Jan 23, 2019, 5:21:18 PM (8 years ago)
Author:
Alan Coon
Message:

Cherry-pick r240024. rdar://problem/47458299

[JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
​https://bugs.webkit.org/show_bug.cgi?id=193438
<rdar://problem/45581249>

Reviewed by Saam Barati and Keith Miller.

JSTests:

Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
Then, GetByVal(String) crashed.

  • stress/string-get-by-val-lowering.js: Added. (shouldBe): (test):
  • stress/type-for-get-by-val-can-be-widen-after-ai.js: Added. (Hello): (foo):

Source/JavaScriptCore:

GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
After the first run removes Check(String), it would happen that AI starts saying the type of 1st child is not String.
To claim that it *is* a String type, we should use KnownStringUse here.

  • dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same problem. This patch fixes it too.
  • dfg/DFGSSALoweringPhase.cpp: (JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
  • ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt): (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@240024 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-607-branch
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-607-branch/JSTests/ChangeLog

    r240371 r240376  
     12019-01-23  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r240024. rdar://problem/47458299
     4
     5    [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
     6    https://bugs.webkit.org/show_bug.cgi?id=193438
     7    <rdar://problem/45581249>
     8   
     9    Reviewed by Saam Barati and Keith Miller.
     10   
     11    JSTests:
     12   
     13    Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
     14    Then, GetByVal(String) crashed.
     15   
     16    * stress/string-get-by-val-lowering.js: Added.
     17    (shouldBe):
     18    (test):
     19    * stress/type-for-get-by-val-can-be-widen-after-ai.js: Added.
     20    (Hello):
     21    (foo):
     22   
     23    Source/JavaScriptCore:
     24   
     25    GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
     26    After the first run removes Check(String), it would happen that AI starts saying the type of 1st child is not String.
     27    To claim that it *is* a String type, we should use KnownStringUse here.
     28   
     29    * dfg/DFGFixupPhase.cpp:
     30    (JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should
     31    change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same
     32    problem. This patch fixes it too.
     33    * dfg/DFGSSALoweringPhase.cpp:
     34    (JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
     35    * ftl/FTLLowerDFGToB3.cpp:
     36    (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
     37    (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
     38   
     39    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240024 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     40
     41    2019-01-15  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
     42
     43            [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
     44            https://bugs.webkit.org/show_bug.cgi?id=193438
     45            <rdar://problem/45581249>
     46
     47            Reviewed by Saam Barati and Keith Miller.
     48
     49            Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
     50            Then, GetByVal(String) crashed.
     51
     52            * stress/string-get-by-val-lowering.js: Added.
     53            (shouldBe):
     54            (test):
     55            * stress/type-for-get-by-val-can-be-widen-after-ai.js: Added.
     56            (Hello):
     57            (foo):
     58
    1592019-01-23  Alan Coon  <alancoon@apple.com>
    260

  • branches/safari-607-branch/Source/JavaScriptCore/ChangeLog

    r240375 r240376  
     12019-01-23  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r240024. rdar://problem/47458299
     4
     5    [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
     6    https://bugs.webkit.org/show_bug.cgi?id=193438
     7    <rdar://problem/45581249>
     8   
     9    Reviewed by Saam Barati and Keith Miller.
     10   
     11    JSTests:
     12   
     13    Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
     14    Then, GetByVal(String) crashed.
     15   
     16    * stress/string-get-by-val-lowering.js: Added.
     17    (shouldBe):
     18    (test):
     19    * stress/type-for-get-by-val-can-be-widen-after-ai.js: Added.
     20    (Hello):
     21    (foo):
     22   
     23    Source/JavaScriptCore:
     24   
     25    GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
     26    After the first run removes Check(String), it would happen that AI starts saying the type of 1st child is not String.
     27    To claim that it *is* a String type, we should use KnownStringUse here.
     28   
     29    * dfg/DFGFixupPhase.cpp:
     30    (JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should
     31    change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same
     32    problem. This patch fixes it too.
     33    * dfg/DFGSSALoweringPhase.cpp:
     34    (JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
     35    * ftl/FTLLowerDFGToB3.cpp:
     36    (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
     37    (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
     38   
     39    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240024 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     40
     41    2019-01-15  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
     42
     43            [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
     44            https://bugs.webkit.org/show_bug.cgi?id=193438
     45            <rdar://problem/45581249>
     46
     47            Reviewed by Saam Barati and Keith Miller.
     48
     49            GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
     50            After the first run removes Check(String), it would happen that AI starts saying the type of 1st child is not String.
     51            To claim that it *is* a String type, we should use KnownStringUse here.
     52
     53            * dfg/DFGFixupPhase.cpp:
     54            (JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should
     55            change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same
     56            problem. This patch fixes it too.
     57            * dfg/DFGSSALoweringPhase.cpp:
     58            (JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
     59            * ftl/FTLLowerDFGToB3.cpp:
     60            (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
     61            (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
     62
    1632019-01-23  Alan Coon  <alancoon@apple.com>
    264

  • branches/safari-607-branch/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

    r240375 r240376  
    778778            ASSERT(node->arrayMode() == ArrayMode(Array::String, Array::Read));
    779779            blessArrayOperation(node->child1(), node->child2(), node->child3());
    780             fixEdge<KnownCellUse>(node->child1());
     780            fixEdge<KnownStringUse>(node->child1());
    781781            fixEdge<Int32Use>(node->child2());
    782782            break;
    … …  
    895895                break;
    896896            case Array::ForceExit:
     897                break;
     898            case Array::String:
     899                fixEdge<KnownStringUse>(m_graph.varArgChild(node, 0));
     900                fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
    897901                break;
    898902            default:

  • branches/safari-607-branch/Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp

    r230144 r240376  
    125125            op = GetVectorLength;
    126126            break;
     127        case Array::String:
     128            // When we need to support this, it will require additional code since base's useKind is KnownStringUse.
     129            DFG_CRASH(m_graph, m_node, "Array::String's base.useKind() is KnownStringUse");
     130            break;
    127131        default:
    128132            break;
    … …  
    131135        Node* length = m_insertionSet.insertNode(
    132136            m_nodeIndex, SpecInt32Only, op, m_node->origin,
    133             OpInfo(m_node->arrayMode().asWord()), base, storage);
     137            OpInfo(m_node->arrayMode().asWord()), Edge(base.node(), KnownCellUse), storage);
    134138        m_insertionSet.insertNode(
    135139            m_nodeIndex, SpecInt32Only, CheckInBounds, m_node->origin,

  • branches/safari-607-branch/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r240375 r240376  
    65886588    void compileStringCharAt()
    65896589    {
    6590         LValue base = lowCell(m_graph.child(m_node, 0));
     6590        LValue base = lowString(m_graph.child(m_node, 0));
    65916591        LValue index = lowInt32(m_graph.child(m_node, 1));
    65926592        LValue storage = lowStorage(m_graph.child(m_node, 2));
    … …  
    67026702        LBasicBlock continuation = m_out.newBlock();
    67036703
    6704         LValue base = lowCell(m_node->child1());
     6704        LValue base = lowString(m_node->child1());
    67056705        LValue index = lowInt32(m_node->child2());
    67066706        LValue storage = lowStorage(m_node->child3());
Note: See TracChangeset for help on using the changeset viewer.