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

Changeset 245919 in webkit


Ignore:
Timestamp:
May 30, 2019, 5:29:55 PM (7 years ago)
Author:
Alan Coon
Message:

Cherry-pick r245071. rdar://problem/51264850

Invalid DFG JIT genereation in high CPU usage state
​https://bugs.webkit.org/show_bug.cgi?id=197453

Reviewed by Saam Barati.

JSTests:

  • stress/string-ident-use-clears-abstract-value-if-rope-string-constant-is-held.js: Added. (trigger): (main):

Source/JavaScriptCore:

We have a DFG graph like this.

a: JSConstant(rope JSString)
b: CheckStringIdent(Check:StringUse:@a)
... AI think this is unreachable ...

When executing StringUse edge filter onto @a, AbstractValue::filterValueByType clears AbstractValue and makes it None.
This is because @a constant produces SpecString (SpecStringVar | SpecStringIdent) while StringUse edge filter requires
SpecStringIdent. AbstractValue::filterValueByType has an assumption that the JS constant always produces the same
SpeculatedType. So it clears AbstractValue completely.
But this assumption is wrong. JSString can produce SpecStringIdent later if the string is resolved to AtomicStringImpl.
AI think that we always fail. But once the string is resolved to AtomicStringImpl, we pass this check. So we execute
the breakpoint emitted by DFG since DFG think this is unreachable.

In this patch, we just clear the m_value if AbstractValue type filter fails with the held constant, since the constant
may produce a narrower type which can meet the type filter later.

  • dfg/DFGAbstractValue.cpp: (JSC::DFG::AbstractValue::filterValueByType):

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

Location:
branches/safari-607-branch
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed

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

    r245918 r245919  
     12019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r245071. rdar://problem/51264850
     4
     5    Invalid DFG JIT genereation in high CPU usage state
     6    https://bugs.webkit.org/show_bug.cgi?id=197453
     7   
     8    Reviewed by Saam Barati.
     9   
     10    JSTests:
     11   
     12    * stress/string-ident-use-clears-abstract-value-if-rope-string-constant-is-held.js: Added.
     13    (trigger):
     14    (main):
     15   
     16    Source/JavaScriptCore:
     17   
     18    We have a DFG graph like this.
     19   
     20        a: JSConstant(rope JSString)
     21        b: CheckStringIdent(Check:StringUse:@a)
     22        ... AI think this is unreachable ...
     23   
     24    When executing StringUse edge filter onto @a, AbstractValue::filterValueByType clears AbstractValue and makes it None.
     25    This is because @a constant produces SpecString (SpecStringVar | SpecStringIdent) while StringUse edge filter requires
     26    SpecStringIdent. AbstractValue::filterValueByType has an assumption that the JS constant always produces the same
     27    SpeculatedType. So it clears AbstractValue completely.
     28    But this assumption is wrong. JSString can produce SpecStringIdent later if the string is resolved to AtomicStringImpl.
     29    AI think that we always fail. But once the string is resolved to AtomicStringImpl, we pass this check. So we execute
     30    the breakpoint emitted by DFG since DFG think this is unreachable.
     31   
     32    In this patch, we just clear the `m_value` if AbstractValue type filter fails with the held constant, since the constant
     33    may produce a narrower type which can meet the type filter later.
     34   
     35    * dfg/DFGAbstractValue.cpp:
     36    (JSC::DFG::AbstractValue::filterValueByType):
     37   
     38    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245071 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     39
     40    2019-05-08  Yusuke Suzuki  <ysuzuki@apple.com>
     41
     42            Invalid DFG JIT genereation in high CPU usage state
     43            https://bugs.webkit.org/show_bug.cgi?id=197453
     44
     45            Reviewed by Saam Barati.
     46
     47            * stress/string-ident-use-clears-abstract-value-if-rope-string-constant-is-held.js: Added.
     48            (trigger):
     49            (main):
     50
    1512019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
    252

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

    r245918 r245919  
     12019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r245071. rdar://problem/51264850
     4
     5    Invalid DFG JIT genereation in high CPU usage state
     6    https://bugs.webkit.org/show_bug.cgi?id=197453
     7   
     8    Reviewed by Saam Barati.
     9   
     10    JSTests:
     11   
     12    * stress/string-ident-use-clears-abstract-value-if-rope-string-constant-is-held.js: Added.
     13    (trigger):
     14    (main):
     15   
     16    Source/JavaScriptCore:
     17   
     18    We have a DFG graph like this.
     19   
     20        a: JSConstant(rope JSString)
     21        b: CheckStringIdent(Check:StringUse:@a)
     22        ... AI think this is unreachable ...
     23   
     24    When executing StringUse edge filter onto @a, AbstractValue::filterValueByType clears AbstractValue and makes it None.
     25    This is because @a constant produces SpecString (SpecStringVar | SpecStringIdent) while StringUse edge filter requires
     26    SpecStringIdent. AbstractValue::filterValueByType has an assumption that the JS constant always produces the same
     27    SpeculatedType. So it clears AbstractValue completely.
     28    But this assumption is wrong. JSString can produce SpecStringIdent later if the string is resolved to AtomicStringImpl.
     29    AI think that we always fail. But once the string is resolved to AtomicStringImpl, we pass this check. So we execute
     30    the breakpoint emitted by DFG since DFG think this is unreachable.
     31   
     32    In this patch, we just clear the `m_value` if AbstractValue type filter fails with the held constant, since the constant
     33    may produce a narrower type which can meet the type filter later.
     34   
     35    * dfg/DFGAbstractValue.cpp:
     36    (JSC::DFG::AbstractValue::filterValueByType):
     37   
     38    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245071 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     39
     40    2019-05-08  Yusuke Suzuki  <ysuzuki@apple.com>
     41
     42            Invalid DFG JIT genereation in high CPU usage state
     43            https://bugs.webkit.org/show_bug.cgi?id=197453
     44
     45            Reviewed by Saam Barati.
     46
     47            We have a DFG graph like this.
     48
     49                a: JSConstant(rope JSString)
     50                b: CheckStringIdent(Check:StringUse:@a)
     51                ... AI think this is unreachable ...
     52
     53            When executing StringUse edge filter onto @a, AbstractValue::filterValueByType clears AbstractValue and makes it None.
     54            This is because @a constant produces SpecString (SpecStringVar | SpecStringIdent) while StringUse edge filter requires
     55            SpecStringIdent. AbstractValue::filterValueByType has an assumption that the JS constant always produces the same
     56            SpeculatedType. So it clears AbstractValue completely.
     57            But this assumption is wrong. JSString can produce SpecStringIdent later if the string is resolved to AtomicStringImpl.
     58            AI think that we always fail. But once the string is resolved to AtomicStringImpl, we pass this check. So we execute
     59            the breakpoint emitted by DFG since DFG think this is unreachable.
     60
     61            In this patch, we just clear the `m_value` if AbstractValue type filter fails with the held constant, since the constant
     62            may produce a narrower type which can meet the type filter later.
     63
     64            * dfg/DFGAbstractValue.cpp:
     65            (JSC::DFG::AbstractValue::filterValueByType):
     66
    1672019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
    268

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

    r240415 r245919  
    342342    // in any realistic scenario, so we don't do it. Simpler is better.
    343343
    344     if (!!m_type) {
    345         // The type is still non-empty. It may be that the new type renders
    346         // the value empty because it contravenes the constant value we had.
    347         if (m_value && !validateType(m_value))
    348             clear();
     344    if (!m_value)
    349345        return;
    350     }
    351    
    352     // The type has been rendered empty. That means that the value must now be invalid,
    353     // as well.
    354     ASSERT(!m_value || !validateType(m_value));
     346
     347    if (validateTypeAcceptingBoxedInt52(m_value))
     348        return;
     349
     350    // We assume that the constant value can produce a narrower type at
     351    // some point. For example, rope JSString produces SpecString, but
     352    // it produces SpecStringIdent once it is resolved to AtomicStringImpl.
     353    // We do not make this AbstractValue cleared, but clear the constant
     354    // value if validation fails currently.
    355355    m_value = JSValue();
    356356}
Note: See TracChangeset for help on using the changeset viewer.