[JSC] Repatch should construct CallCases and CasesValue at the same time
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Aug 2019 03:28:20 +0000 (03:28 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Aug 2019 03:28:20 +0000 (03:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201325

Reviewed by Saam Barati.

JSTests:

* stress/repatch-switch.js: Added.
(main.f2.f0):
(main.f2.f3):
(main.f2.f1):
(main.f2):
(main):

Source/JavaScriptCore:

In linkPolymorphicCall, we should create callCases and casesValue at the same time to assert `callCases.size() == casesValue.size()`.
If the call variant is isClosureCall and InternalFunction, we skip adding it to casesValue. So we should not add this variant to callCases too.

* jit/Repatch.cpp:
(JSC::linkPolymorphicCall):

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

JSTests/ChangeLog
JSTests/stress/repatch-switch.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jit/Repatch.cpp

index ebd636c..1d917f0 100644 (file)
@@ -1,5 +1,19 @@
 2019-08-29  Yusuke Suzuki  <ysuzuki@apple.com>
 
+        [JSC] Repatch should construct CallCases and CasesValue at the same time
+        https://bugs.webkit.org/show_bug.cgi?id=201325
+
+        Reviewed by Saam Barati.
+
+        * stress/repatch-switch.js: Added.
+        (main.f2.f0):
+        (main.f2.f3):
+        (main.f2.f1):
+        (main.f2):
+        (main):
+
+2019-08-29  Yusuke Suzuki  <ysuzuki@apple.com>
+
         [JSC] ObjectAllocationSinkingPhase wrongly deals with always-taken branches during interpretation
         https://bugs.webkit.org/show_bug.cgi?id=198650
 
diff --git a/JSTests/stress/repatch-switch.js b/JSTests/stress/repatch-switch.js
new file mode 100644 (file)
index 0000000..a70abfc
--- /dev/null
@@ -0,0 +1,87 @@
+//@ runDefault("--useConcurrentJIT=0")
+function main() {
+    const u0 = {a: 0};
+    const u3 = {
+        d: 0.1,
+        eeeee: 0,
+        abcdefghijklm: 0,
+    };
+
+    const u4 = {
+        abcdefghijklm: 0,
+    };
+
+    function f2() {
+        for (let i = 0; i < 10; i++) {
+            const o0 = { a: 0 };
+            [0];
+            const o1 = {};
+            const u6 = {
+                a: 0,
+                b: -2,
+                log10: 0,
+                copyWithin: 0,
+                values: -20
+            };
+            const s0 = 'aaabcdef';
+            function f0() {
+                const u8 = {
+                    abcde: 20,
+                };
+                for (const u9 of s0) {
+                }
+                String.fromCharCode([0], 256, 0);
+                const u9 = {caaaaa: 0};
+                o1[-4000000000] = WeakMap;
+                const f4 = Function.bind(0, 0);
+                new Promise(f4);
+            }
+            const p2 = {
+                call: 0,
+                construct: 0,
+                getPrototypeOf: 0,
+                has: 0,
+                defineProperty: 0,
+                isExtensible: 0,
+            };
+            new Promise(f0);
+            function f3(ay) {
+                new Promise(WeakSet);
+            }
+            const u12 = new Promise(f3);
+            function f1(ax) {
+                'use strict';
+                ax.xa = f1;
+            }
+            if (i != 0) {
+                let c0 = 0;
+                while (c0 < 6) {
+                    let c1 = 0;
+                    while (c1 < 8) {
+                        c1++;
+                    }
+                    c0++;
+                }
+                f1(o0, 0);
+            } else {
+                o0.xy = 0;
+                o0.xx = {q:0};
+            }
+            for (let c2 = 0; c2 < 100; c2++) {}
+            const u11 = [0, 0, 1.1];
+            const o2 = { a: 0 };
+            for (let c3 = 0; c3 < 100; c3++) {
+                o2.toString;
+            }
+            o0.xyz;
+        }
+        print;
+    }
+    f2();
+    f2();
+
+    Math.atan();
+    new Uint8Array();
+}
+
+main();
index ff29f93..cc038c5 100644 (file)
@@ -1,5 +1,18 @@
 2019-08-29  Yusuke Suzuki  <ysuzuki@apple.com>
 
+        [JSC] Repatch should construct CallCases and CasesValue at the same time
+        https://bugs.webkit.org/show_bug.cgi?id=201325
+
+        Reviewed by Saam Barati.
+
+        In linkPolymorphicCall, we should create callCases and casesValue at the same time to assert `callCases.size() == casesValue.size()`.
+        If the call variant is isClosureCall and InternalFunction, we skip adding it to casesValue. So we should not add this variant to callCases too.
+
+        * jit/Repatch.cpp:
+        (JSC::linkPolymorphicCall):
+
+2019-08-29  Yusuke Suzuki  <ysuzuki@apple.com>
+
         [JSC] ObjectAllocationSinkingPhase wrongly deals with always-taken branches during interpretation
         https://bugs.webkit.org/show_bug.cgi?id=198650
 
index a5175ca..2c4c565 100644 (file)
@@ -1007,6 +1007,7 @@ void linkPolymorphicCall(
         callLinkInfo.setHasSeenClosure();
     
     Vector<PolymorphicCallCase> callCases;
+    Vector<int64_t> caseValues;
     
     // Figure out what our cases are.
     for (CallVariant variant : list) {
@@ -1021,36 +1022,7 @@ void linkPolymorphicCall(
                 return;
             }
         }
-        
-        callCases.append(PolymorphicCallCase(variant, codeBlock));
-    }
-    
-    // If we are over the limit, just use a normal virtual call.
-    unsigned maxPolymorphicCallVariantListSize;
-    if (isWebAssembly)
-        maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSizeForWebAssemblyToJS();
-    else if (callerCodeBlock->jitType() == JITCode::topTierJIT())
-        maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSizeForTopTier();
-    else
-        maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSize();
 
-    if (list.size() > maxPolymorphicCallVariantListSize) {
-        linkVirtualFor(exec, callLinkInfo);
-        return;
-    }
-
-    Vector<int64_t> caseValues(callCases.size());
-    Vector<CallToCodePtr> calls(callCases.size());
-    UniqueArray<uint32_t> fastCounts;
-    
-    if (!isWebAssembly && callerCodeBlock->jitType() != JITCode::topTierJIT())
-        fastCounts = makeUniqueArray<uint32_t>(callCases.size());
-    
-    for (size_t i = 0; i < callCases.size(); ++i) {
-        if (fastCounts)
-            fastCounts[i] = 0;
-        
-        CallVariant variant = callCases[i].variant();
         int64_t newCaseValue = 0;
         if (isClosureCall) {
             newCaseValue = bitwise_cast<intptr_t>(variant.executable());
@@ -1064,25 +1036,47 @@ void linkPolymorphicCall(
             else
                 newCaseValue = bitwise_cast<intptr_t>(variant.internalFunction());
         }
-        
-        if (!ASSERT_DISABLED) {
-            for (size_t j = 0; j < i; ++j) {
-                if (caseValues[j] != newCaseValue)
-                    continue;
 
+        if (!ASSERT_DISABLED) {
+            if (caseValues.contains(newCaseValue)) {
                 dataLog("ERROR: Attempt to add duplicate case value.\n");
                 dataLog("Existing case values: ");
                 CommaPrinter comma;
-                for (size_t k = 0; k < i; ++k)
-                    dataLog(comma, caseValues[k]);
+                for (auto& value : caseValues)
+                    dataLog(comma, value);
                 dataLog("\n");
                 dataLog("Attempting to add: ", newCaseValue, "\n");
                 dataLog("Variant list: ", listDump(callCases), "\n");
                 RELEASE_ASSERT_NOT_REACHED();
             }
         }
-        
-        caseValues[i] = newCaseValue;
+
+        callCases.append(PolymorphicCallCase(variant, codeBlock));
+        caseValues.append(newCaseValue);
+    }
+    ASSERT(callCases.size() == caseValues.size());
+
+    // If we are over the limit, just use a normal virtual call.
+    unsigned maxPolymorphicCallVariantListSize;
+    if (isWebAssembly)
+        maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSizeForWebAssemblyToJS();
+    else if (callerCodeBlock->jitType() == JITCode::topTierJIT())
+        maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSizeForTopTier();
+    else
+        maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSize();
+
+    // We use list.size() instead of callCases.size() because we respect CallVariant size for now.
+    if (list.size() > maxPolymorphicCallVariantListSize) {
+        linkVirtualFor(exec, callLinkInfo);
+        return;
+    }
+
+    Vector<CallToCodePtr> calls(callCases.size());
+    UniqueArray<uint32_t> fastCounts;
+
+    if (!isWebAssembly && callerCodeBlock->jitType() != JITCode::topTierJIT()) {
+        fastCounts = makeUniqueArray<uint32_t>(callCases.size());
+        memset(fastCounts.get(), 0, callCases.size() * sizeof(uint32_t));
     }
     
     GPRReg calleeGPR = callLinkInfo.calleeGPR();