Bug 230972 - We need to load the baseline JIT's constant pool register after OSR exit to checkpoints if we return to baseline code
Summary: We need to load the baseline JIT's constant pool register after OSR exit to c...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-29 11:41 PDT by Saam Barati
Modified: 2021-09-29 18:05 PDT (History)
7 users (show)

See Also:


Attachments
patch (9.19 KB, patch)
2021-09-29 11:51 PDT, Saam Barati
mark.lam: review+
Details | Formatted Diff | Diff
patch for landing (9.34 KB, patch)
2021-09-29 12:25 PDT, Saam Barati
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2021-09-29 11:41:53 PDT
Consider this:
- 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, 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.
Comment 1 Saam Barati 2021-09-29 11:42:27 PDT
<rdar://83659469>
Comment 2 Saam Barati 2021-09-29 11:51:10 PDT
Created attachment 439635 [details]
patch
Comment 3 Mark Lam 2021-09-29 12:13:05 PDT
Comment on attachment 439635 [details]
patch

r=me
Comment 4 Yusuke Suzuki 2021-09-29 12:17:34 PDT
Comment on attachment 439635 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=439635&action=review

r=me too

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:2501
> +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:

We need to have `if JIT` thing before accessing to CodeBlock::m_jitData to ensure that LLInt without JIT configuration works. (Not using CLoop, but not using JIT).
Comment 5 Saam Barati 2021-09-29 12:25:10 PDT
Created attachment 439644 [details]
patch for landing
Comment 6 EWS 2021-09-29 17:47:49 PDT
Committed r283288 (242315@main): <https://commits.webkit.org/242315@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439644 [details].