Title: [186702] trunk/Source/_javascript_Core
- Revision
- 186702
- Author
- [email protected]
- Date
- 2015-07-10 20:01:20 -0700 (Fri, 10 Jul 2015)
Log Message
AI folding of IsObjectOrNull is broken for non-object types that may be null
https://bugs.webkit.org/show_bug.cgi?id=146867
Reviewed by Ryosuke Niwa.
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): Fix the bug and add some text describing what is going on.
* tests/stress/misc-is-object-or-null.js: Added. Test for the bug.
(foo):
* tests/stress/other-is-object-or-null.js: Added. Test for a bug I almost introduced.
(foo):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (186701 => 186702)
--- trunk/Source/_javascript_Core/ChangeLog 2015-07-11 02:30:04 UTC (rev 186701)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-07-11 03:01:20 UTC (rev 186702)
@@ -1,5 +1,19 @@
2015-07-10 Filip Pizlo <[email protected]>
+ AI folding of IsObjectOrNull is broken for non-object types that may be null
+ https://bugs.webkit.org/show_bug.cgi?id=146867
+
+ Reviewed by Ryosuke Niwa.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): Fix the bug and add some text describing what is going on.
+ * tests/stress/misc-is-object-or-null.js: Added. Test for the bug.
+ (foo):
+ * tests/stress/other-is-object-or-null.js: Added. Test for a bug I almost introduced.
+ (foo):
+
+2015-07-10 Filip Pizlo <[email protected]>
+
It should be easy to measure total compile times.
https://bugs.webkit.org/show_bug.cgi?id=146857
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (186701 => 186702)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2015-07-11 02:30:04 UTC (rev 186701)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2015-07-11 03:01:20 UTC (rev 186702)
@@ -961,6 +961,10 @@
break;
}
+ // FIXME: This code should really use AbstractValue::isType() and
+ // AbstractValue::couldBeType().
+ // https://bugs.webkit.org/show_bug.cgi?id=146870
+
bool constantWasSet = false;
switch (node->op()) {
case IsUndefined:
@@ -1034,13 +1038,28 @@
// FIXME: Use the masquerades-as-undefined watchpoint thingy.
// https://bugs.webkit.org/show_bug.cgi?id=144456
+ // These expressions are complicated to parse. A helpful way to parse this is that
+ // "!(T & ~S)" means "T is a subset of S". Conversely, "!(T & S)" means "T is a
+ // disjoint set from S". Things like "T - S" means that, provided that S is a
+ // subset of T, it's the "set of all things in T but not in S". Things like "T | S"
+ // mean the "union of T and S".
+
+ // Is the child's type an object that isn't an other-object (i.e. object that could
+ // have masquaredes-as-undefined traps) and isn't a function? Then: we should fold
+ // this to true.
if (!(child.m_type & ~(SpecObject - SpecObjectOther - SpecFunction))) {
setConstant(node, jsBoolean(true));
constantWasSet = true;
break;
}
- if (!(child.m_type & (SpecObject - SpecFunction))) {
+ // Is the child's type definitely not either of: an object that isn't a function,
+ // or either undefined or null? Then: we should fold this to false. This means
+ // for example that if it's any non-function object, including those that have
+ // masquerades-as-undefined traps, then we don't fold. It also means we won't fold
+ // if it's undefined-or-null, since the type bits don't distinguish between
+ // undefined (which should fold to false) and null (which should fold to true).
+ if (!(child.m_type & ((SpecObject - SpecFunction) | SpecOther))) {
setConstant(node, jsBoolean(false));
constantWasSet = true;
break;
Added: trunk/Source/_javascript_Core/tests/stress/misc-is-object-or-null.js (0 => 186702)
--- trunk/Source/_javascript_Core/tests/stress/misc-is-object-or-null.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/misc-is-object-or-null.js 2015-07-11 03:01:20 UTC (rev 186702)
@@ -0,0 +1,13 @@
+function foo(p) {
+ var x = p ? null : false;
+ return (typeof x) == "object";
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var p = !!(i & 1);
+ var result = foo(p);
+ if (result !== p)
+ throw "Error: bad result for p = " + p + ": " + result;
+}
Added: trunk/Source/_javascript_Core/tests/stress/other-is-object-or-null.js (0 => 186702)
--- trunk/Source/_javascript_Core/tests/stress/other-is-object-or-null.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/other-is-object-or-null.js 2015-07-11 03:01:20 UTC (rev 186702)
@@ -0,0 +1,13 @@
+function foo(p) {
+ var x = p ? null : void 0;
+ return (typeof x) == "object";
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var p = !!(i & 1);
+ var result = foo(p);
+ if (result !== p)
+ throw "Error: bad result for p = " + p + ": " + result;
+}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
