Title: [286278] trunk
Revision
286278
Author
sbar...@apple.com
Date
2021-11-29 17:58:37 -0800 (Mon, 29 Nov 2021)

Log Message

FTL's implementation of HasIndexedProperty for InBounds accesses checks the inverse of what it should be checking when exiting by seeing a hole
https://bugs.webkit.org/show_bug.cgi?id=233408
<rdar://problem/85787251>

Reviewed by Mark Lam.

JSTests:

* stress/in-by-val-has-indexed-property-ftl-3.js: Added.
* stress/in-by-val-has-indexed-property-ftl-2.js: Added.
* stress/in-by-val-has-indexed-property-ftl.js: Added.

Source/_javascript_Core:

The implementation of an InBounds HasIndexedProperty in FTL, when speculating, we
would exit when we did not see a hole, not when we did see a hole. This is
the inverse of what we need to do, we should exit when we do see a hole.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
* tools/JSDollarVM.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
(JSC::JSDollarVM::finishCreation):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (286277 => 286278)


--- trunk/JSTests/ChangeLog	2021-11-30 01:42:49 UTC (rev 286277)
+++ trunk/JSTests/ChangeLog	2021-11-30 01:58:37 UTC (rev 286278)
@@ -1,3 +1,15 @@
+2021-11-29  Saam Barati  <sbar...@apple.com>
+
+        FTL's implementation of HasIndexedProperty for InBounds accesses checks the inverse of what it should be checking when exiting by seeing a hole
+        https://bugs.webkit.org/show_bug.cgi?id=233408
+        <rdar://problem/85787251>
+
+        Reviewed by Mark Lam.
+
+        * stress/in-by-val-has-indexed-property-ftl-3.js: Added.
+        * stress/in-by-val-has-indexed-property-ftl-2.js: Added.
+        * stress/in-by-val-has-indexed-property-ftl.js: Added.
+
 2021-11-29  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] slice should be aware of TerminationException

Added: trunk/JSTests/stress/in-by-val-has-indexed-property-ftl-2.js (0 => 286278)


--- trunk/JSTests/stress/in-by-val-has-indexed-property-ftl-2.js	                        (rev 0)
+++ trunk/JSTests/stress/in-by-val-has-indexed-property-ftl-2.js	2021-11-30 01:58:37 UTC (rev 286278)
@@ -0,0 +1,28 @@
+//@ runDefault("--validateOptions=true", "--useConcurrentJIT=false", "--useConcurrentGC=false", "--validateBCE=true", "--thresholdForJITSoon=1", "--thresholdForJITAfterWarmUp=7", "--thresholdForOptimizeAfterWarmUp=7", "--thresholdForOptimizeAfterLongWarmUp=7", "--thresholdForOptimizeSoon=1", "--thresholdForFTLOptimizeAfterWarmUp=10")
+
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+
+function main() {
+    let v17 = {__proto__:[42,1]};
+    v17[2] = 4;
+        
+    let v92 = 0;
+    for (let v95 = 0; v95 < 100; v95++) {
+        function doEvery(e, i) {
+            assert(e === 42);
+            assert(i === 0);
+            function doMap() {
+                v139 = v92++;
+            }   
+            noInline(doMap);
+            [0].map(doMap);
+        }   
+        noInline(doEvery);
+        v17.every(doEvery);
+    }   
+    assert(v139 === 99);
+}
+main();

Added: trunk/JSTests/stress/in-by-val-has-indexed-property-ftl-3.js (0 => 286278)


--- trunk/JSTests/stress/in-by-val-has-indexed-property-ftl-3.js	                        (rev 0)
+++ trunk/JSTests/stress/in-by-val-has-indexed-property-ftl-3.js	2021-11-30 01:58:37 UTC (rev 286278)
@@ -0,0 +1,97 @@
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+
+function test1() {
+    function func(b, o) {
+        if (b)
+            return 2 in o;
+        return false;
+    }
+    noInline(func);
+
+    let o = {__proto__:[0, 1]};
+    o[3] = 42;
+
+    for (let i = 0; i < 100; ++i) {
+        func(true, o);
+        func(false, o);
+    }
+
+    for (let i = 0; i < 10000; ++i) {
+        assert(!func(false, o));
+    }
+    assert(!func(true, o));
+}
+test1();
+
+function test2() {
+    function func(b, o) {
+        if (b)
+            return 2 in o;
+        return false;
+    }
+    noInline(func);
+
+    let o = {__proto__:[0, 1]};
+    o[3] = {};
+
+    for (let i = 0; i < 100; ++i) {
+        func(true, o);
+        func(false, o);
+    }
+
+    for (let i = 0; i < 10000; ++i) {
+        assert(!func(false, o));
+    }
+    assert(!func(true, o));
+}
+test2();
+
+function test3() {
+    function func(b, o) {
+        if (b)
+            return 2 in o;
+        return false;
+    }
+    noInline(func);
+
+    let o = {__proto__:[0, 1]};
+    o[3] = 42.2;
+
+    for (let i = 0; i < 100; ++i) {
+        func(true, o);
+        func(false, o);
+    }
+
+    for (let i = 0; i < 10000; ++i) {
+        assert(!func(false, o));
+    }
+    assert(!func(true, o));
+}
+test3();
+
+function test4() {
+    function func(b, o) {
+        if (b)
+            return 2 in o;
+        return false;
+    }
+    noInline(func);
+
+    let o = {__proto__:[0, 1]};
+    o[3] = {};
+    $vm.ensureArrayStorage(o);
+
+    for (let i = 0; i < 100; ++i) {
+        func(true, o);
+        func(false, o);
+    }
+
+    for (let i = 0; i < 10000; ++i) {
+        assert(!func(false, o));
+    }
+    assert(!func(true, o));
+}
+test4();

Added: trunk/JSTests/stress/in-by-val-has-indexed-property-ftl.js (0 => 286278)


--- trunk/JSTests/stress/in-by-val-has-indexed-property-ftl.js	                        (rev 0)
+++ trunk/JSTests/stress/in-by-val-has-indexed-property-ftl.js	2021-11-30 01:58:37 UTC (rev 286278)
@@ -0,0 +1,94 @@
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+
+function test1() {
+    function func(b, o) {
+        if (b)
+            return 0 in o;
+        return true;
+    }
+    noInline(func);
+
+    let o = {__proto__:[0, 1]};
+    o[2] = 4;
+    for (let i = 0; i < 100; ++i) {
+        func(true, o);
+        func(false, o);
+    }
+
+    for (let i = 0; i < 10000; ++i) {
+        assert(func(false, o));
+    }
+    assert(func(true, o));
+}
+test1();
+
+function test2() {
+    function func(b, o) {
+        if (b)
+            return 0 in o;
+        return true;
+    }
+    noInline(func);
+
+    let o = {__proto__:[0, 1]};
+    o[2] = 13.333;
+    for (let i = 0; i < 100; ++i) {
+        func(true, o);
+        func(false, o);
+    }
+
+    for (let i = 0; i < 10000; ++i) {
+        assert(func(false, o));
+    }
+    assert(func(true, o));
+}
+test2();
+
+function test3() {
+    function func(b, o) {
+        if (b)
+            return 0 in o;
+        return true;
+    }
+    noInline(func);
+
+    let o = {__proto__:[0, 1]};
+    o[2] = {};
+    for (let i = 0; i < 100; ++i) {
+        func(true, o);
+        func(false, o);
+    }
+
+    for (let i = 0; i < 10000; ++i) {
+        assert(func(false, o));
+    }
+    assert(func(true, o));
+}
+test3();
+
+function test4() {
+    function func(b, o) {
+        if (b)
+            return 0 in o;
+        return true;
+    }
+    noInline(func);
+
+    let o = {__proto__:[0, 1]};
+    o[2] = {};
+    $vm.ensureArrayStorage(o);
+
+    for (let i = 0; i < 100; ++i) {
+        func(true, o);
+        func(false, o);
+    }
+
+    for (let i = 0; i < 10000; ++i) {
+        assert(func(false, o));
+    }
+    assert(func(true, o));
+}
+test4();

Modified: trunk/Source/_javascript_Core/ChangeLog (286277 => 286278)


--- trunk/Source/_javascript_Core/ChangeLog	2021-11-30 01:42:49 UTC (rev 286277)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-11-30 01:58:37 UTC (rev 286278)
@@ -1,3 +1,21 @@
+2021-11-29  Saam Barati  <sbar...@apple.com>
+
+        FTL's implementation of HasIndexedProperty for InBounds accesses checks the inverse of what it should be checking when exiting by seeing a hole
+        https://bugs.webkit.org/show_bug.cgi?id=233408
+        <rdar://problem/85787251>
+
+        Reviewed by Mark Lam.
+
+        The implementation of an InBounds HasIndexedProperty in FTL, when speculating, we
+        would exit when we did not see a hole, not when we did see a hole. This is
+        the inverse of what we need to do, we should exit when we do see a hole.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
+        * tools/JSDollarVM.cpp:
+        (JSC::JSC_DEFINE_HOST_FUNCTION):
+        (JSC::JSDollarVM::finishCreation):
+
 2021-11-29  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] slice should be aware of TerminationException

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (286277 => 286278)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2021-11-30 01:42:49 UTC (rev 286277)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2021-11-30 01:58:37 UTC (rev 286278)
@@ -13322,15 +13322,15 @@
             } else
                 lastNext = m_out.insertNewBlocksBefore(slowCase);
 
-            LValue checkHoleResultValue =
-                m_out.notZero64(m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1))));
-            ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
+            LValue isHole =
+                m_out.isZero64(m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1))));
+            ValueFromBlock checkHoleResult = m_out.anchor(m_out.logicalNot(isHole));
             if (mode.isInBoundsSaneChain())
                 m_out.jump(continuation);
             else if (!mode.isInBounds())
-                m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+                m_out.branch(isHole, rarely(slowCase), usually(continuation));
             else
-                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
+                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, isHole);
 
             m_out.appendTo(slowCase, continuation);
             ValueFromBlock slowResult = m_out.anchor(
@@ -13360,14 +13360,14 @@
                 lastNext = m_out.insertNewBlocksBefore(slowCase);
 
             LValue doubleValue = m_out.loadDouble(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1)));
-            LValue checkHoleResultValue = m_out.doubleEqual(doubleValue, doubleValue);
-            ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
+            LValue notHole = m_out.doubleEqual(doubleValue, doubleValue);
+            ValueFromBlock checkHoleResult = m_out.anchor(notHole);
             if (mode.isInBoundsSaneChain())
                 m_out.jump(continuation);
             else if (!mode.isInBounds())
-                m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+                m_out.branch(notHole, usually(continuation), rarely(slowCase));
             else
-                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
+                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, m_out.logicalNot(notHole));
             
             m_out.appendTo(slowCase, continuation);
             ValueFromBlock slowResult = m_out.anchor(
@@ -13395,15 +13395,15 @@
             } else
                 lastNext = m_out.insertNewBlocksBefore(slowCase);
 
-            LValue checkHoleResultValue =
-                m_out.notZero64(m_out.load64(baseIndex(m_heaps.ArrayStorage_vector, storage, index, m_graph.varArgChild(m_node, 1))));
-            ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
+            LValue isHole =
+                m_out.isZero64(m_out.load64(baseIndex(m_heaps.ArrayStorage_vector, storage, index, m_graph.varArgChild(m_node, 1))));
+            ValueFromBlock checkHoleResult = m_out.anchor(m_out.logicalNot(isHole));
             if (mode.isInBoundsSaneChain())
                 m_out.jump(continuation);
             else if (!mode.isInBounds())
-                m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+                m_out.branch(isHole, rarely(slowCase), usually(continuation));
             else
-                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
+                speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, isHole);
 
             m_out.appendTo(slowCase, continuation);
             ValueFromBlock slowResult = m_out.anchor(

Modified: trunk/Source/_javascript_Core/tools/JSDollarVM.cpp (286277 => 286278)


--- trunk/Source/_javascript_Core/tools/JSDollarVM.cpp	2021-11-30 01:42:49 UTC (rev 286277)
+++ trunk/Source/_javascript_Core/tools/JSDollarVM.cpp	2021-11-30 01:58:37 UTC (rev 286278)
@@ -2132,6 +2132,8 @@
 static JSC_DECLARE_HOST_FUNCTION(functionResetJITSizeStatistics);
 #endif
 
+static JSC_DECLARE_HOST_FUNCTION(functionEnsureArrayStorage);
+
 const ClassInfo JSDollarVM::s_info = { "DollarVM", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSDollarVM) };
 
 static EncodedJSValue doPrint(JSGlobalObject* globalObject, CallFrame* callFrame, bool addLineFeed)
@@ -3826,6 +3828,18 @@
 }
 #endif
 
+JSC_DEFINE_HOST_FUNCTION(functionEnsureArrayStorage, (JSGlobalObject* globalObject, CallFrame* callFrame))
+{
+    DollarVMAssertScope assertScope;
+
+    VM& vm = globalObject->vm();
+
+    JSValue arg = callFrame->argument(0);
+    if (arg.isObject())
+        asObject(arg)->ensureArrayStorage(vm);
+    return JSValue::encode(jsUndefined());
+}
+
 constexpr unsigned jsDollarVMPropertyAttributes = PropertyAttribute::ReadOnly | PropertyAttribute::DontEnum | PropertyAttribute::DontDelete;
 
 void JSDollarVM::finishCreation(VM& vm)
@@ -3996,6 +4010,8 @@
     addFunction(vm, "resetJITSizeStatistics", functionResetJITSizeStatistics, 0);
 #endif
 
+    addFunction(vm, "ensureArrayStorage", functionEnsureArrayStorage, 1);
+
     m_objectDoingSideEffectPutWithoutCorrectSlotStatusStructure.set(vm, this, ObjectDoingSideEffectPutWithoutCorrectSlotStatus::createStructure(vm, globalObject, jsNull()));
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to