REGRESSION(r227341 and r227742): AI and clobberize should be precise and consistent...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Apr 2018 17:57:29 +0000 (17:57 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Apr 2018 17:57:29 +0000 (17:57 +0000)
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):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGClobberize.h

index fa5f655..2e72796 100644 (file)
@@ -1,3 +1,31 @@
+2018-04-10  Filip Pizlo  <fpizlo@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  <fpizlo@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
index 238c004..804b47d 100644 (file)
@@ -1636,7 +1636,7 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
             }
         }
 
-        if (node->child1().useKind() == UntypedUse || node->child2().useKind() == UntypedUse)
+        if (node->isBinaryUseKind(UntypedUse))
             clobberWorld(node->origin.semantic, clobberLimit);
         forNode(node).setType(SpecBoolean);
         break;
index 1ec4556..4278d94 100644 (file)
@@ -1537,13 +1537,8 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
             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;