Title: [245919] branches/safari-607-branch
- Revision
- 245919
- Author
- [email protected]
- Date
- 2019-05-30 17:29:55 -0700 (Thu, 30 May 2019)
Log 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/_javascript_Core:
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
Modified Paths
Added Paths
Diff
Modified: branches/safari-607-branch/JSTests/ChangeLog (245918 => 245919)
--- branches/safari-607-branch/JSTests/ChangeLog 2019-05-31 00:29:53 UTC (rev 245918)
+++ branches/safari-607-branch/JSTests/ChangeLog 2019-05-31 00:29:55 UTC (rev 245919)
@@ -1,5 +1,55 @@
2019-05-30 Kocsen Chung <[email protected]>
+ 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/_javascript_Core:
+
+ 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
+
+ 2019-05-08 Yusuke Suzuki <[email protected]>
+
+ Invalid DFG JIT genereation in high CPU usage state
+ https://bugs.webkit.org/show_bug.cgi?id=197453
+
+ Reviewed by Saam Barati.
+
+ * stress/string-ident-use-clears-abstract-value-if-rope-string-constant-is-held.js: Added.
+ (trigger):
+ (main):
+
+2019-05-30 Kocsen Chung <[email protected]>
+
Cherry-pick r245018. rdar://problem/51264878
tryCachePutByID should not crash if target offset changes
Added: branches/safari-607-branch/JSTests/stress/string-ident-use-clears-abstract-value-if-rope-string-constant-is-held.js (0 => 245919)
--- branches/safari-607-branch/JSTests/stress/string-ident-use-clears-abstract-value-if-rope-string-constant-is-held.js (rev 0)
+++ branches/safari-607-branch/JSTests/stress/string-ident-use-clears-abstract-value-if-rope-string-constant-is-held.js 2019-05-31 00:29:55 UTC (rev 245919)
@@ -0,0 +1,20 @@
+//@ runDefault("--useConcurrentJIT=0")
+function trigger() {
+ var v21 = "test";
+ var v25 = (10)[11];
+ var v27 = 4294967297 + v21;
+ for (let v31 = 1; v31 < 1000; v31++) {
+ ;
+ }
+ v33 = new Float32Array();
+ v34 = v21[v27];
+ v35 = v33[v27];
+}
+
+function main() {
+ for (let i = 0; i < 10000; i++) {
+ trigger();
+ }
+}
+
+main();
Modified: branches/safari-607-branch/Source/_javascript_Core/ChangeLog (245918 => 245919)
--- branches/safari-607-branch/Source/_javascript_Core/ChangeLog 2019-05-31 00:29:53 UTC (rev 245918)
+++ branches/safari-607-branch/Source/_javascript_Core/ChangeLog 2019-05-31 00:29:55 UTC (rev 245919)
@@ -1,5 +1,71 @@
2019-05-30 Kocsen Chung <[email protected]>
+ 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/_javascript_Core:
+
+ 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
+
+ 2019-05-08 Yusuke Suzuki <[email protected]>
+
+ Invalid DFG JIT genereation in high CPU usage state
+ https://bugs.webkit.org/show_bug.cgi?id=197453
+
+ Reviewed by Saam Barati.
+
+ 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):
+
+2019-05-30 Kocsen Chung <[email protected]>
+
Cherry-pick r245018. rdar://problem/51264878
tryCachePutByID should not crash if target offset changes
Modified: branches/safari-607-branch/Source/_javascript_Core/dfg/DFGAbstractValue.cpp (245918 => 245919)
--- branches/safari-607-branch/Source/_javascript_Core/dfg/DFGAbstractValue.cpp 2019-05-31 00:29:53 UTC (rev 245918)
+++ branches/safari-607-branch/Source/_javascript_Core/dfg/DFGAbstractValue.cpp 2019-05-31 00:29:55 UTC (rev 245919)
@@ -341,17 +341,17 @@
// the value, then we could clear both of those things. But that's unlikely to help
// in any realistic scenario, so we don't do it. Simpler is better.
- if (!!m_type) {
- // The type is still non-empty. It may be that the new type renders
- // the value empty because it contravenes the constant value we had.
- if (m_value && !validateType(m_value))
- clear();
+ if (!m_value)
return;
- }
-
- // The type has been rendered empty. That means that the value must now be invalid,
- // as well.
- ASSERT(!m_value || !validateType(m_value));
+
+ if (validateTypeAcceptingBoxedInt52(m_value))
+ return;
+
+ // We assume that the constant value can produce a narrower type at
+ // some point. For example, rope JSString produces SpecString, but
+ // it produces SpecStringIdent once it is resolved to AtomicStringImpl.
+ // We do not make this AbstractValue cleared, but clear the constant
+ // value if validation fails currently.
m_value = JSValue();
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
