Changeset 249310 in webkit

Timestamp:

Aug 29, 2019 8:28:20 PM (5 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] Repatch should construct CallCases and CasesValue at the same time
​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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/repatch-switch.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/jit/Repatch.cpp (modified) (4 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/jit/Repatch.cpp

@@ -1008 +1008 @@
     
     Vector<PolymorphicCallCase> callCases;
+    Vector<int64_t> caseValues;
     
     // Figure out what our cases are.
@@ -1022 +1023 @@
             }
         }
-        
-        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) {
@@ -1065 +1037 @@
                 newCaseValue = bitwise_cast<intptr_t>(variant.internalFunction());
         }
-        
+
         if (!ASSERT_DISABLED) {
-            for (size_t j = 0; j < i; ++j) {
-                if (caseValues[j] != newCaseValue)
-                    continue;
-
+            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");
@@ -1082 +1051 @@
             }
         }
-        
-        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));
     }