KnownCellUse should also have SpecCellCheck as its type filter
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Nov 2018 20:42:51 +0000 (20:42 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Nov 2018 20:42:51 +0000 (20:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191729
<rdar://problem/45872852>

Reviewed by Filip Pizlo.

JSTests:

* stress/known-cell-type-check-should-allow-empty-value-to-flow-through.js: Added.
(C):

Source/JavaScriptCore:

We write transformations in the compiler like this where we emit edges with
KnownCellUse if we know we're inserting code at a point where we're dominated
by a Cell check:

a: SomeValue
b: Something(Cell:@a)
c: SomethingElse(@b)
d: CheckNotEmpty(@a)

=>

a: SomeValue
b: Something(Cell:@a)
e: RandomOtherThing(KnownCellUse:@a)
c: SomethingElse(@b)
d: CheckNotEmpty(@a)

However, doing this used to lead to subtly incorrect programs since KnownCellUse
did not allow the empty value to flow through it. We used to end up incorrectly
deleting @d in the above program. We fix this, we make KnownCellUse allow the empty
value to flow through.

* dfg/DFGUseKind.h:
(JSC::DFG::typeFilterFor):

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

JSTests/ChangeLog
JSTests/stress/known-cell-type-check-should-allow-empty-value-to-flow-through.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGUseKind.h

index a94844d..7317115 100644 (file)
@@ -1,3 +1,14 @@
+2018-11-16  Saam Barati  <sbarati@apple.com>
+
+        KnownCellUse should also have SpecCellCheck as its type filter
+        https://bugs.webkit.org/show_bug.cgi?id=191729
+        <rdar://problem/45872852>
+
+        Reviewed by Filip Pizlo.
+
+        * stress/known-cell-type-check-should-allow-empty-value-to-flow-through.js: Added.
+        (C):
+
 2018-11-16  Tadeu Zagallo  <tzagallo@apple.com>
 
         Fix assertion failure on BytecodeGenerator::recordOpcode
diff --git a/JSTests/stress/known-cell-type-check-should-allow-empty-value-to-flow-through.js b/JSTests/stress/known-cell-type-check-should-allow-empty-value-to-flow-through.js
new file mode 100644 (file)
index 0000000..067f85f
--- /dev/null
@@ -0,0 +1,25 @@
+//@ runDefault("--jitPolicyScale=0", "--useConcurrentJIT=0")
+
+class C extends class {} {
+    constructor(beforeSuper) {
+        let f = () => {
+            for (let j=0; j<10; j++) {
+                try {
+                    this.x
+                } catch (e) {
+                }
+            }
+        };
+        if (beforeSuper) {
+            f();
+            super();
+        } else {
+            super();
+            f();
+        }
+    }
+};
+for (let i = 0; i < 10000; i++) {
+    new C(false);
+    new C(true);
+}
index 6acd241..46f6912 100644 (file)
@@ -1,3 +1,36 @@
+2018-11-16  Saam Barati  <sbarati@apple.com>
+
+        KnownCellUse should also have SpecCellCheck as its type filter
+        https://bugs.webkit.org/show_bug.cgi?id=191729
+        <rdar://problem/45872852>
+
+        Reviewed by Filip Pizlo.
+
+        We write transformations in the compiler like this where we emit edges with
+        KnownCellUse if we know we're inserting code at a point where we're dominated
+        by a Cell check:
+        
+        a: SomeValue
+        b: Something(Cell:@a)
+        c: SomethingElse(@b)
+        d: CheckNotEmpty(@a)
+        
+        =>
+        
+        a: SomeValue
+        b: Something(Cell:@a)
+        e: RandomOtherThing(KnownCellUse:@a)
+        c: SomethingElse(@b)
+        d: CheckNotEmpty(@a)
+        
+        However, doing this used to lead to subtly incorrect programs since KnownCellUse
+        did not allow the empty value to flow through it. We used to end up incorrectly
+        deleting @d in the above program. We fix this, we make KnownCellUse allow the empty
+        value to flow through.
+
+        * dfg/DFGUseKind.h:
+        (JSC::DFG::typeFilterFor):
+
 2018-11-16  Tadeu Zagallo  <tzagallo@apple.com>
 
         Fix assertion failure on BytecodeGenerator::recordOpcode
index d898ecb..9fb17cc 100644 (file)
@@ -119,9 +119,8 @@ inline SpeculatedType typeFilterFor(UseKind useKind)
     case KnownBooleanUse:
         return SpecBoolean;
     case CellUse:
-        return SpecCellCheck;
     case KnownCellUse:
-        return SpecCell;
+        return SpecCellCheck;
     case CellOrOtherUse:
         return SpecCellCheck | SpecOther;
     case ObjectUse: