Remove Broken CompareEq constant folding phase.
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Jul 2017 00:45:58 +0000 (00:45 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Jul 2017 00:45:58 +0000 (00:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174846
<rdar://problem/32978808>

Reviewed by Saam Barati.

This bug happened when we would get code like the following:

a: JSConst(Undefined)
b: GetLocal(SomeObjectOrUndefined)
...
c: CompareEq(Check:ObjectOrOther:b, Check:ObjectOrOther:a)

constant folding will turn this into:

a: JSConst(Undefined)
b: GetLocal(SomeObjectOrUndefined)
...
c: CompareEq(Check:ObjectOrOther:b, Other:a)

But the SpeculativeJIT/FTL lowering will fail to check b
properly which leads to an assertion failure in the AI.

I'll follow up with a more robust fix later. For now, I'll remove the
case that generates the code. Removing the code appears to be perf
neutral.

* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

index c9b0209..d74172e 100644 (file)
@@ -1,3 +1,35 @@
+2017-07-25  Keith Miller  <keith_miller@apple.com>
+
+        Remove Broken CompareEq constant folding phase.
+        https://bugs.webkit.org/show_bug.cgi?id=174846
+        <rdar://problem/32978808>
+
+        Reviewed by Saam Barati.
+
+        This bug happened when we would get code like the following:
+
+        a: JSConst(Undefined)
+        b: GetLocal(SomeObjectOrUndefined)
+        ...
+        c: CompareEq(Check:ObjectOrOther:b, Check:ObjectOrOther:a)
+
+        constant folding will turn this into:
+
+        a: JSConst(Undefined)
+        b: GetLocal(SomeObjectOrUndefined)
+        ...
+        c: CompareEq(Check:ObjectOrOther:b, Other:a)
+
+        But the SpeculativeJIT/FTL lowering will fail to check b
+        properly which leads to an assertion failure in the AI.
+
+        I'll follow up with a more robust fix later. For now, I'll remove the
+        case that generates the code. Removing the code appears to be perf
+        neutral.
+
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::foldConstants):
+
 2017-07-25  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: Refactoring: extract async stack trace logic from InspectorInstrumentation
index 8f4a455..7345cd4 100644 (file)
@@ -135,10 +135,8 @@ private:
             }
 
             case CompareEq: {
-                if (!m_interpreter.needsTypeCheck(node->child1(), SpecOther))
-                    node->child1().setUseKind(OtherUse);
-                if (!m_interpreter.needsTypeCheck(node->child2(), SpecOther))
-                    node->child2().setUseKind(OtherUse);
+                // FIXME: We should add back the broken folding phase here for comparisions where we prove at least one side has type SpecOther.
+                // See: https://bugs.webkit.org/show_bug.cgi?id=174844
                 break;
             }