Title: [236824] trunk
Revision
236824
Author
sbar...@apple.com
Date
2018-10-03 20:29:57 -0700 (Wed, 03 Oct 2018)

Log Message

lowXYZ in FTLLower should always filter the type of the incoming edge
https://bugs.webkit.org/show_bug.cgi?id=189939
<rdar://problem/44407030>

Reviewed by Michael Saboff.

JSTests:

* stress/ftl-should-always-filter-for-low-type-check-functions.js: Added.
(foo):
(test):

Source/_javascript_Core:

For example, the FTL may know more about data flow than AI in certain programs,
and it needs to inform AI of these data flow properties to appease the assertion
we have in AI that a node must perform type checks on its child nodes.

For example, consider this program:

```
bb#1
a: Phi // Let's say it has an Int32 result, so it goes into the int32 hash table in FTLLower
Branch(...,  #2, #3)

bb#2
ArrayifyToStructure(Cell:@a) // This modifies @a to have the its previous type union the type of some structure set.
Jump(#3)

bb#3
c: Add(Int32:@something, Int32:@a)
```

When the Add node does lowInt32() for @a, FTL lower used to just grab it
from the int32 hash table without filtering the AbstractValue. However,
the parent node is asking for a type check to happen, so we must inform
AI of this "type check" if we want to appease the assertion that all nodes
perform type checks for their edges that semantically perform type checks.
This patch makes it so we filter the AbstractValue in the lowXYZ even
if FTLLower proved the value must be XYZ.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compilePhi):
(JSC::FTL::DFG::LowerDFGToB3::simulatedTypeCheck):
(JSC::FTL::DFG::LowerDFGToB3::lowInt32):
(JSC::FTL::DFG::LowerDFGToB3::lowCell):
(JSC::FTL::DFG::LowerDFGToB3::lowBoolean):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (236823 => 236824)


--- trunk/JSTests/ChangeLog	2018-10-04 03:27:20 UTC (rev 236823)
+++ trunk/JSTests/ChangeLog	2018-10-04 03:29:57 UTC (rev 236824)
@@ -1,3 +1,15 @@
+2018-10-03  Saam barati  <sbar...@apple.com>
+
+        lowXYZ in FTLLower should always filter the type of the incoming edge
+        https://bugs.webkit.org/show_bug.cgi?id=189939
+        <rdar://problem/44407030>
+
+        Reviewed by Michael Saboff.
+
+        * stress/ftl-should-always-filter-for-low-type-check-functions.js: Added.
+        (foo):
+        (test):
+
 2018-10-03  Mark Lam  <mark....@apple.com>
 
         Make string MaxLength for all WTF and JS strings consistently equal to INT_MAX.

Added: trunk/JSTests/stress/ftl-should-always-filter-for-low-type-check-functions.js (0 => 236824)


--- trunk/JSTests/stress/ftl-should-always-filter-for-low-type-check-functions.js	                        (rev 0)
+++ trunk/JSTests/stress/ftl-should-always-filter-for-low-type-check-functions.js	2018-10-04 03:29:57 UTC (rev 236824)
@@ -0,0 +1,31 @@
+//@ runDefault("--useConcurrentJIT=0", "--jitPolicyScale=0", "--maximumInliningDepth=2")
+
+function foo(x, y) {
+    var w = 0;
+    for (var i = 0; i < x.length; ++i) {
+        for (var j = 0; j < x.length; ++j)
+            w += foo(j, i);
+        y[i] = w;
+    }
+}
+
+function test(x, a3) {
+      a1 = [];
+      a2 = [];
+
+    for (i = 0; i < x; ++i)
+        a1[i] = 0;
+
+    for (i = 0; i < 10; ++i) {
+        foo(a3, a2);
+        foo(a3, a1);
+    }
+}
+noDFG(test);
+
+a3 = [];
+for (var i = 0; i < 3; ++i)
+    a3[i] = 0;
+
+for (var i = 3; i <= 12; i *= 2)
+    test(i, a3);

Modified: trunk/Source/_javascript_Core/ChangeLog (236823 => 236824)


--- trunk/Source/_javascript_Core/ChangeLog	2018-10-04 03:27:20 UTC (rev 236823)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-10-04 03:29:57 UTC (rev 236824)
@@ -1,3 +1,45 @@
+2018-10-03  Saam barati  <sbar...@apple.com>
+
+        lowXYZ in FTLLower should always filter the type of the incoming edge
+        https://bugs.webkit.org/show_bug.cgi?id=189939
+        <rdar://problem/44407030>
+
+        Reviewed by Michael Saboff.
+
+        For example, the FTL may know more about data flow than AI in certain programs,
+        and it needs to inform AI of these data flow properties to appease the assertion
+        we have in AI that a node must perform type checks on its child nodes.
+        
+        For example, consider this program:
+        
+        ```
+        bb#1
+        a: Phi // Let's say it has an Int32 result, so it goes into the int32 hash table in FTLLower
+        Branch(...,  #2, #3)
+        
+        bb#2
+        ArrayifyToStructure(Cell:@a) // This modifies @a to have the its previous type union the type of some structure set.
+        Jump(#3)
+        
+        bb#3
+        c: Add(Int32:@something, Int32:@a)
+        ```
+        
+        When the Add node does lowInt32() for @a, FTL lower used to just grab it
+        from the int32 hash table without filtering the AbstractValue. However,
+        the parent node is asking for a type check to happen, so we must inform
+        AI of this "type check" if we want to appease the assertion that all nodes
+        perform type checks for their edges that semantically perform type checks.
+        This patch makes it so we filter the AbstractValue in the lowXYZ even
+        if FTLLower proved the value must be XYZ.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compilePhi):
+        (JSC::FTL::DFG::LowerDFGToB3::simulatedTypeCheck):
+        (JSC::FTL::DFG::LowerDFGToB3::lowInt32):
+        (JSC::FTL::DFG::LowerDFGToB3::lowCell):
+        (JSC::FTL::DFG::LowerDFGToB3::lowBoolean):
+
 2018-10-03  Michael Saboff  <msab...@apple.com>
 
         Command line jsc should report memory footprint in bytes

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (236823 => 236824)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-10-04 03:27:20 UTC (rev 236823)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-10-04 03:29:57 UTC (rev 236824)
@@ -1416,7 +1416,7 @@
             setJSValue(phi);
             break;
         default:
-            DFG_CRASH(m_graph, m_node, "Bad use kind");
+            DFG_CRASH(m_graph, m_node, "Bad result type");
             break;
         }
     }
@@ -14627,6 +14627,11 @@
     {
         m_state.setIsValid(false);
     }
+
+    void simulatedTypeCheck(Edge highValue, SpeculatedType typesPassedThrough)
+    {
+        m_interpreter.filter(highValue, typesPassedThrough);
+    }
     
     void typeCheck(
         FormattedValue lowValue, Edge highValue, SpeculatedType typesPassedThrough,
@@ -14652,6 +14657,7 @@
         
         if (edge->hasConstant()) {
             JSValue value = edge->asJSValue();
+            simulatedTypeCheck(edge, SpecInt32Only);
             if (!value.isInt32()) {
                 if (mayHaveTypeCheck(edge.useKind()))
                     terminate(Uncountable);
@@ -14663,8 +14669,10 @@
         }
         
         LoweredNodeValue value = m_int32Values.get(edge.node());
-        if (isValid(value))
+        if (isValid(value)) {
+            simulatedTypeCheck(edge, SpecInt32Only);
             return value.value();
+        }
         
         value = m_strictInt52Values.get(edge.node());
         if (isValid(value))
@@ -14772,6 +14780,7 @@
         
         if (edge->op() == JSConstant) {
             FrozenValue* value = edge->constant();
+            simulatedTypeCheck(edge, SpecCellCheck);
             if (!value->value().isCell()) {
                 if (mayHaveTypeCheck(edge.useKind()))
                     terminate(Uncountable);
@@ -14899,6 +14908,7 @@
         
         if (edge->hasConstant()) {
             JSValue value = edge->asJSValue();
+            simulatedTypeCheck(edge, SpecBoolean);
             if (!value.isBoolean()) {
                 if (mayHaveTypeCheck(edge.useKind()))
                     terminate(Uncountable);
@@ -14910,8 +14920,10 @@
         }
         
         LoweredNodeValue value = m_booleanValues.get(edge.node());
-        if (isValid(value))
+        if (isValid(value)) {
+            simulatedTypeCheck(edge, SpecBoolean);
             return value.value();
+        }
         
         value = m_jsValueValues.get(edge.node());
         if (isValid(value)) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to