Changeset 283288 in webkit

Timestamp:

Sep 29, 2021, 5:47:41 PM (5 years ago)

Author:

sbarati@apple.com

Message:

We need to load the baseline JIT's constant pool register after OSR exit to checkpoints if we return to baseline code
​https://bugs.webkit.org/show_bug.cgi?id=230972
<rdar://83659469>

Reviewed by Mark Lam and Yusuke Suzuki.

JSTests:

• stress/checkpoint-osr-exit-needs-to-reload-baseline-jit-constant-pool-gpr.js: Added.

(empty):
(empty2):
(test):

Source/JavaScriptCore:

Consider the following:

• We have a CodeBlock A.
• DFG or FTL compiles an exit to A when A is still LLInt code. This means the OSR exit code will materialize registers as if A is LLInt.
• We tier up A to Baseline JIT code.
• Now, we take the exit to A as if it's LLInt. But the checkpoint OSR exit code will actually jump to the tiered up baseline code when it's done, because it determines where to jump at runtime. Because of this, when we return from the checkpoint code, and if we are jumping into baseline code, we must always load the constant pool register.
• There's no need to load the metadata register because that register is shared with LLInt code, and will already contain the right value.

• jit/JIT.cpp:

(JSC::JIT::privateCompileMainPass):

• llint/LLIntSlowPaths.cpp:

(JSC::LLInt::dispatchToNextInstructionDuringExit):
(JSC::LLInt::llint_slow_path_checkpoint_osr_exit_from_inlined_call):
(JSC::LLInt::llint_slow_path_checkpoint_osr_exit):
(JSC::LLInt::dispatchToNextInstruction): Deleted.

• llint/LowLevelInterpreter.asm:
• llint/LowLevelInterpreter64.asm:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/checkpoint-osr-exit-needs-to-reload-baseline-jit-constant-pool-gpr.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/jit/JIT.cpp (modified) (1 diff)
• Source/JavaScriptCore/llint/LLIntSlowPaths.cpp (modified) (4 diffs)
• Source/JavaScriptCore/llint/LowLevelInterpreter.asm (modified) (3 diffs)
• Source/JavaScriptCore/llint/LowLevelInterpreter64.asm (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-09-29  Saam Barati  <sbarati@apple.com>
+
+        We need to load the baseline JIT's constant pool register after OSR exit to checkpoints if we return to baseline code
+        https://bugs.webkit.org/show_bug.cgi?id=230972
+        <rdar://83659469>
+
+        Reviewed by Mark Lam and Yusuke Suzuki.
+
+        * stress/checkpoint-osr-exit-needs-to-reload-baseline-jit-constant-pool-gpr.js: Added.
+        (empty):
+        (empty2):
+        (test):
+
 2021-09-29  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-09-29  Saam Barati  <sbarati@apple.com>
+
+        We need to load the baseline JIT's constant pool register after OSR exit to checkpoints if we return to baseline code
+        https://bugs.webkit.org/show_bug.cgi?id=230972
+        <rdar://83659469>
+
+        Reviewed by Mark Lam and Yusuke Suzuki.
+
+        Consider the following:
+        - We have a CodeBlock A.
+        - DFG or FTL compiles an exit to A when A is still LLInt code. This means
+          the OSR exit code will materialize registers as if A is LLInt.
+        - We tier up A to Baseline JIT code.
+        - Now, we take the exit to A as if it's LLInt. But the checkpoint OSR exit
+          code will actually jump to the tiered up baseline code when it's done,
+          because it determines where to jump at runtime. Because of this, when
+          we return from the checkpoint code, and if we are jumping into baseline
+          code, we must always load the constant pool register.
+        - There's no need to load the metadata register because that register is
+          shared with LLInt code, and will already contain the right value.
+
+        * jit/JIT.cpp:
+        (JSC::JIT::privateCompileMainPass):
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::dispatchToNextInstructionDuringExit):
+        (JSC::LLInt::llint_slow_path_checkpoint_osr_exit_from_inlined_call):
+        (JSC::LLInt::llint_slow_path_checkpoint_osr_exit):
+        (JSC::LLInt::dispatchToNextInstruction): Deleted.
+        * llint/LowLevelInterpreter.asm:
+        * llint/LowLevelInterpreter64.asm:
+
 2021-09-29  Basuke Suzuki  <basuke.suzuki@sony.com>
 

trunk/Source/JavaScriptCore/jit/JIT.cpp

@@ -270 +270 @@
             sizeMarker = m_vm->jitSizeStatistics->markStart(id, *this);
         }
+
+#if ASSERT_ENABLED
+        if (opcodeID != op_catch) {
+            probeDebug([=] (Probe::Context& ctx) {
+                CodeBlock* codeBlock = ctx.fp<CallFrame*>()->codeBlock();
+                auto* constantPool = ctx.gpr<void*>(s_constantsGPR);
+                RELEASE_ASSERT(codeBlock->baselineJITConstantPool() == constantPool);
+                auto* metadata = ctx.gpr<void*>(s_metadataGPR);
+                RELEASE_ASSERT(codeBlock->metadataTable() == metadata);
+            });
+        }
+#endif
 
         if (UNLIKELY(m_compilation)) {

trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

@@ -2410 +2410 @@
 }
 
-inline SlowPathReturnType dispatchToNextInstruction(ThrowScope& scope, CodeBlock* codeBlock, InstructionStream::Ref pc)
+inline SlowPathReturnType dispatchToNextInstructionDuringExit(ThrowScope& scope, CodeBlock* codeBlock, InstructionStream::Ref pc)
 {
     if (scope.exception())
@@ -2428 +2428 @@
     BytecodeIndex nextBytecodeIndex = pc.next().index();
     auto nextBytecode = codeBlock->jitCodeMap().find(nextBytecodeIndex);
-    return encodeResult(nullptr, nextBytecode.executableAddress());
+    return encodeResult(bitwise_cast<void*>(static_cast<uintptr_t>(1)), nextBytecode.executableAddress());
 #endif
     RELEASE_ASSERT_NOT_REACHED();
@@ -2480 +2480 @@
     }
 
-    return dispatchToNextInstruction(scope, codeBlock, pc);
+    return dispatchToNextInstructionDuringExit(scope, codeBlock, pc);
 }
 
@@ -2525 +2525 @@
     }
 
-    return dispatchToNextInstruction(scope, codeBlock, pc);
+    return dispatchToNextInstructionDuringExit(scope, codeBlock, pc);
 }
 

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm

@@ -2483 +2483 @@
 
 
+if JIT
+    macro loadBaselineJITConstantPool()
+        # Baseline uses LLInt's PB register for its JIT constant pool.
+        loadp CodeBlock[cfr], PB
+        loadp CodeBlock::m_jitData[PB], PB
+        loadp CodeBlock::JITData::m_jitConstantPool[PB], PB
+    end
+
+    macro setupReturnToBaselineAfterCheckpointExitIfNeeded()
+        # DFG or FTL OSR exit could have compiled an OSR exit to LLInt code.
+        # That means it set up registers as if execution would happen in the
+        # LLInt. However, during OSR exit for checkpoints, we might return to
+        # JIT code if it's already compiled. After the OSR exit gets compiled,
+        # we can tier up to JIT code. And checkpoint exit will jump to it.
+        # That means we always need to set up our constant pool GPR, because the OSR
+        # exit code might not have done it.
+        bpneq r0, 1, .notBaselineJIT
+        loadBaselineJITConstantPool()
+    .notBaselineJIT:
+
+    end
+else
+    macro setupReturnToBaselineAfterCheckpointExitIfNeeded()
+    end
+end
+
 op(checkpoint_osr_exit_from_inlined_call_trampoline, macro ()
     if (JSVALUE64 and not (C_LOOP or C_LOOP_WIN)) or ARMv7 or MIPS
@@ -2506 +2532 @@
         end
 
+        setupReturnToBaselineAfterCheckpointExitIfNeeded()
         restoreStateAfterCCall()
         branchIfException(_llint_throw_from_slow_path_trampoline)
+
         if ARM64E
             move r1, a0
@@ -2529 +2557 @@
         # We don't call saveStateForCCall() because we are going to use the bytecodeIndex from our side state.
         cCall2(_llint_slow_path_checkpoint_osr_exit)
+        setupReturnToBaselineAfterCheckpointExitIfNeeded()
         restoreStateAfterCCall()
         branchIfException(_llint_throw_from_slow_path_trampoline)

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

@@ -445 +445 @@
                 move r1, sp
 
-                # Baseline uses LLInt's PB register for its JIT constant pool.
-                loadp CodeBlock[cfr], PB
-                loadp CodeBlock::m_jitData[PB], PB
-                loadp CodeBlock::JITData::m_jitConstantPool[PB], PB
+                loadBaselineJITConstantPool()
 
                 if ARM64E