JSTests:
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 21 Jan 2019 03:54:17 +0000 (03:54 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 21 Jan 2019 03:54:17 +0000 (03:54 +0000)
MovHint must merge NodeBytecodeUsesAsValue for its child
https://bugs.webkit.org/show_bug.cgi?id=186916
<rdar://problem/41396612>

Reviewed by Yusuke Suzuki.

* stress/arith-abs-to-arith-negate-range-optimizaton.js:
* stress/movhint-backwards-propagation-must-merge-use-as-value.js: Added.

Source/JavaScriptCore:
MovHint must merge NodeBytecodeUsesAsValue for its child in backwards propagation
https://bugs.webkit.org/show_bug.cgi?id=186916
<rdar://problem/41396612>

Reviewed by Yusuke Suzuki.

Otherwise, we may not think we care about the non-integral part in
a division (or perhaps overflow in an add, etc). Consider a program
like this:

```return a / b```

That gets compiled to:
```
a: ArithDiv // We don't check that the remainder is zero here.
b: MovHint(@a)
c: ForceOSRExit
d: Unreachable
```

If we don't inform @a that we care about its result in full number
accuracy, it will choose to ignore its non-integral remainder. This
makes sense if *everybody* that all uses of the Div only cared about
the integral part. However, OSR exit is not one of those users. OSR
exit cares about the fractional bits in such a Div.

* dfg/DFGBackwardsPropagationPhase.cpp:
(JSC::DFG::BackwardsPropagationPhase::propagate):

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

JSTests/ChangeLog
JSTests/stress/arith-abs-to-arith-negate-range-optimizaton.js
JSTests/stress/movhint-backwards-propagation-must-merge-use-as-value-add.js [new file with mode: 0644]
JSTests/stress/movhint-backwards-propagation-must-merge-use-as-value.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp

index d07e674..d211705 100644 (file)
@@ -1,3 +1,14 @@
+2019-01-20  Saam Barati  <sbarati@apple.com>
+
+        MovHint must merge NodeBytecodeUsesAsValue for its child
+        https://bugs.webkit.org/show_bug.cgi?id=186916
+        <rdar://problem/41396612>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/arith-abs-to-arith-negate-range-optimizaton.js:
+        * stress/movhint-backwards-propagation-must-merge-use-as-value.js: Added.
+
 2019-01-20  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] Invalidate old scope operations using global lexical binding epoch
index be22972..faeeae6 100644 (file)
@@ -233,7 +233,7 @@ function testUncheckedBetweenIntMinInclusiveAndZeroExclusive()
             throw "Failed testUncheckedBetweenIntMinInclusiveAndZeroExclusive() on -2147483648";
         }
     }
-    if (numberOfDFGCompiles(opaqueUncheckedBetweenIntMinInclusiveAndZeroExclusive) > 1) {
+    if (numberOfDFGCompiles(opaqueUncheckedBetweenIntMinInclusiveAndZeroExclusive) > 2) {
         throw "Failed optimizing testUncheckedBetweenIntMinInclusiveAndZeroExclusive(). None of the tested case need to OSR Exit.";
     }
 }
@@ -376,7 +376,7 @@ function testUncheckedLessThanZero()
             throw "Failed testUncheckedLessThanOrEqualZero() on -2147483648";
         }
     }
-    if (numberOfDFGCompiles(opaqueUncheckedLessThanZero) > 1) {
+    if (numberOfDFGCompiles(opaqueUncheckedLessThanZero) > 2) {
         throw "Failed optimizing testUncheckedLessThanZero(). None of the tested case need to OSR Exit.";
     }
 
@@ -420,7 +420,7 @@ function testUncheckedLessThanOrEqualZero()
             throw "Failed testUncheckedLessThanOrEqualZero() on -2147483648";
         }
     }
-    if (numberOfDFGCompiles(opaqueUncheckedLessThanOrEqualZero) > 1) {
+    if (numberOfDFGCompiles(opaqueUncheckedLessThanOrEqualZero) > 2) {
         throw "Failed optimizing testUncheckedLessThanOrEqualZero(). None of the tested case need to OSR Exit.";
     }
 }
diff --git a/JSTests/stress/movhint-backwards-propagation-must-merge-use-as-value-add.js b/JSTests/stress/movhint-backwards-propagation-must-merge-use-as-value-add.js
new file mode 100644 (file)
index 0000000..2330488
--- /dev/null
@@ -0,0 +1,16 @@
+function foo(v, a, b) {
+    if (v) {
+        let r = a + b;
+        OSRExit();
+        return r;
+    }
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; ++i) {
+    let r = foo(true, 4, 4);
+    if (r !== 8)
+        throw new Error("Bad!");
+}
+if (foo(true, 2**31 - 1, 1) !== 2**31)
+    throw new Error("Bad!");
diff --git a/JSTests/stress/movhint-backwards-propagation-must-merge-use-as-value.js b/JSTests/stress/movhint-backwards-propagation-must-merge-use-as-value.js
new file mode 100644 (file)
index 0000000..09772de
--- /dev/null
@@ -0,0 +1,16 @@
+function foo(v, a, b) {
+    if (v) {
+        let r = a / b;
+        OSRExit();
+        return r;
+    }
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; ++i) {
+    let r = foo(true, 4, 4);
+    if (r !== 1)
+        throw new Error("Bad!");
+}
+if (foo(true, 1, 4) !== 0.25)
+    throw new Error("Bad!");
index 40275a8..1c74eae 100644 (file)
@@ -1,3 +1,34 @@
+2019-01-20  Saam Barati  <sbarati@apple.com>
+
+        MovHint must merge NodeBytecodeUsesAsValue for its child in backwards propagation
+        https://bugs.webkit.org/show_bug.cgi?id=186916
+        <rdar://problem/41396612>
+
+        Reviewed by Yusuke Suzuki.
+
+        Otherwise, we may not think we care about the non-integral part in
+        a division (or perhaps overflow in an add, etc). Consider a program
+        like this:
+        
+        ```return a / b```
+        
+        That gets compiled to:
+        ```
+        a: ArithDiv // We don't check that the remainder is zero here.
+        b: MovHint(@a)
+        c: ForceOSRExit
+        d: Unreachable
+        ```
+        
+        If we don't inform @a that we care about its result in full number
+        accuracy, it will choose to ignore its non-integral remainder. This
+        makes sense if *everybody* that all uses of the Div only cared about
+        the integral part. However, OSR exit is not one of those users. OSR
+        exit cares about the fractional bits in such a Div.
+
+        * dfg/DFGBackwardsPropagationPhase.cpp:
+        (JSC::DFG::BackwardsPropagationPhase::propagate):
+
 2019-01-20  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] Invalidate old scope operations using global lexical binding epoch
index f4ce71e..e98296f 100644 (file)
@@ -205,7 +205,6 @@ private:
             break;
         }
             
-        case MovHint:
         case Check:
         case CheckVarargs:
             break;