Title: [231409] releases/WebKitGTK/webkit-2.20/Source/_javascript_Core
Revision
231409
Author
[email protected]
Date
2018-05-07 00:54:08 -0700 (Mon, 07 May 2018)

Log Message

Merge r230485 - 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: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog (231408 => 231409)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog	2018-05-07 07:46:37 UTC (rev 231408)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog	2018-05-07 07:54:08 UTC (rev 231409)
@@ -1,3 +1,31 @@
+2018-04-10  Filip Pizlo  <[email protected]>
+
+        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-04  Filip Pizlo  <[email protected]>
 
         REGRESSION(r222563): removed DoubleReal type check causes tons of crashes because CSE has never known how to handle SaneChain

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (231408 => 231409)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2018-05-07 07:46:37 UTC (rev 231408)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2018-05-07 07:54:08 UTC (rev 231409)
@@ -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: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGClobberize.h (231408 => 231409)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGClobberize.h	2018-05-07 07:46:37 UTC (rev 231408)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGClobberize.h	2018-05-07 07:54:08 UTC (rev 231409)
@@ -1557,13 +1557,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
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to