Title: [230485] trunk/Source/_javascript_Core
- Revision
- 230485
- Author
- fpi...@apple.com
- Date
- 2018-04-10 10:57:29 -0700 (Tue, 10 Apr 2018)
Log Message
REGRESSION(r227341 and r227742): AI and clobberize should be precise and consistent about the effectfulness of CompareEq
https://bugs.webkit.org/show_bug.cgi?id=184455
Reviewed by Michael Saboff.
LICM is sort of an assertion that AI is as precise as clobberize about effects. If clobberize
says that something is not effectful, then LICM will try to hoist it. But LICM's AI hack
(AtTailAbstractState) cannot handle hoisting of things that have effects. So, if AI thinks that
the thing being hoisted does have effects, then we get a crash.
In r227341, we incorrectly told AI that CompareEq(Untyped:, _) is effectful. In fact, only
ComapreEq(Untyped:, Untyped:) is effectful, and clobberize knew this already. As a result, LICM
would blow up if we hoisted CompareEq(Untyped:, Other:), which clobberize knew wasn't
effectful.
Instead of fixing this by making AI precise, in r227742 we made matters worse by then breaking
clobberize to also think that CompareEq(Untyped:, _) is effectful.
This fixes the whole situation by teaching both clobberize and AI that the only effectful form
of CompareEq is ComapreEq(Untyped:, Untyped:).
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (230484 => 230485)
--- trunk/Source/_javascript_Core/ChangeLog 2018-04-10 17:21:50 UTC (rev 230484)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-04-10 17:57:29 UTC (rev 230485)
@@ -1,3 +1,31 @@
+2018-04-10 Filip Pizlo <fpi...@apple.com>
+
+ REGRESSION(r227341 and r227742): AI and clobberize should be precise and consistent about the effectfulness of CompareEq
+ https://bugs.webkit.org/show_bug.cgi?id=184455
+
+ Reviewed by Michael Saboff.
+
+ LICM is sort of an assertion that AI is as precise as clobberize about effects. If clobberize
+ says that something is not effectful, then LICM will try to hoist it. But LICM's AI hack
+ (AtTailAbstractState) cannot handle hoisting of things that have effects. So, if AI thinks that
+ the thing being hoisted does have effects, then we get a crash.
+
+ In r227341, we incorrectly told AI that CompareEq(Untyped:, _) is effectful. In fact, only
+ ComapreEq(Untyped:, Untyped:) is effectful, and clobberize knew this already. As a result, LICM
+ would blow up if we hoisted CompareEq(Untyped:, Other:), which clobberize knew wasn't
+ effectful.
+
+ Instead of fixing this by making AI precise, in r227742 we made matters worse by then breaking
+ clobberize to also think that CompareEq(Untyped:, _) is effectful.
+
+ This fixes the whole situation by teaching both clobberize and AI that the only effectful form
+ of CompareEq is ComapreEq(Untyped:, Untyped:).
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+
2018-04-09 Filip Pizlo <fpi...@apple.com>
Executing known edge types may reveal a contradiction causing us to emit an exit at a node that is not allowed to exit
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (230484 => 230485)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-04-10 17:21:50 UTC (rev 230484)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-04-10 17:57:29 UTC (rev 230485)
@@ -1636,7 +1636,7 @@
}
}
- if (node->child1().useKind() == UntypedUse || node->child2().useKind() == UntypedUse)
+ if (node->isBinaryUseKind(UntypedUse))
clobberWorld(node->origin.semantic, clobberLimit);
forNode(node).setType(SpecBoolean);
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (230484 => 230485)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2018-04-10 17:21:50 UTC (rev 230484)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2018-04-10 17:57:29 UTC (rev 230485)
@@ -1537,13 +1537,8 @@
write(HeapObjectCount);
return;
}
-
- if (node->op() == CompareEq && node->isBinaryUseKind(ObjectUse)) {
- def(PureValue(node));
- return;
- }
- if (node->child1().useKind() == UntypedUse || node->child1().useKind() == ObjectUse
- || node->child2().useKind() == UntypedUse || node->child2().useKind() == ObjectUse) {
+
+ if (node->isBinaryUseKind(UntypedUse)) {
read(World);
write(Heap);
return;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
