Bug 235388

Summary: [JSC] Fix non-JIT Windows LLInt
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
mark.lam: review+
Patch
none
Patch none

Description Yusuke Suzuki 2022-01-19 16:51:24 PST
[JSC] Fix non-JIT Windows LLInt
Comment 1 Yusuke Suzuki 2022-01-19 16:53:03 PST
Created attachment 449531 [details]
Patch
Comment 2 Yusuke Suzuki 2022-01-19 16:57:29 PST
Created attachment 449532 [details]
Patch
Comment 3 Mark Lam 2022-01-19 17:29:20 PST
Comment on attachment 449532 [details]
Patch

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

r=me with fixes.

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:131
> +    elsif C_LOOP or C_LOOP_WIN
> +        cloopCallSlowPath3 function, a0, a1, a2

This should come first because C_LOOP can have X86 or X86_WIN enabled too (or any other CPU arch).  I'm surprised that it works for cCall2 (don't know why).  Maybe should fix that too for consistency.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:165
> +        addp 64, sp

Maybe this `addp` should come after the register moves below.  r0 is still pointing to the reserved stack position, right?  They should remain protected by sp until after we move the results out into registers.
Comment 4 Yusuke Suzuki 2022-01-19 17:30:39 PST
Comment on attachment 449532 [details]
Patch

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

Thank you!

>> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:131
>> +        cloopCallSlowPath3 function, a0, a1, a2
> 
> This should come first because C_LOOP can have X86 or X86_WIN enabled too (or any other CPU arch).  I'm surprised that it works for cCall2 (don't know why).  Maybe should fix that too for consistency.

OK, done :)

>> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:165
>> +        addp 64, sp
> 
> Maybe this `addp` should come after the register moves below.  r0 is still pointing to the reserved stack position, right?  They should remain protected by sp until after we move the results out into registers.

Sounds good, done :)
Comment 5 Yusuke Suzuki 2022-01-19 17:34:10 PST
Created attachment 449534 [details]
Patch
Comment 6 Yusuke Suzuki 2022-01-19 17:36:04 PST
Created attachment 449535 [details]
Patch
Comment 7 EWS 2022-01-19 20:04:21 PST
Committed r288268 (246210@main): <https://commits.webkit.org/246210@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449535 [details].
Comment 8 Radar WebKit Bug Importer 2022-01-20 12:07:43 PST
<rdar://problem/87842831>