Changeset 219895 in webkit

Timestamp:

Jul 25, 2017 5:45:58 PM (7 years ago)

Author:

keith_miller@apple.com

Message:

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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGConstantFoldingPhase.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

@@ -136 +136 @@
 
             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;
             }