Bug 127086

Summary: CStack: Fix 32-bit C loop LLINT.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, mhahnenberg, msaboff, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 127071    
Bug Blocks:    
Attachments:
Description Flags
the patch. ggaren: review+

Description Mark Lam 2014-01-16 00:46:53 PST
Patch coming.
Comment 1 Mark Lam 2014-01-16 01:01:29 PST
Created attachment 221343 [details]
the patch.

This patch builds and passes run-javascriptcore-tests --debug --32-bit --cloop --no-jsc-stress.  This is the minimum patch that will build and run the 32-bit C loop LLINT.  Since the C loop is based on ARM, this patch also fixes part of the ARMv7 LLINT implementation.  There is still more to do for ARM (e.g. alignment issues) but I didn’t want to overlap with the work in Michael’s patch.  There is still necessarily some minimal overlap with Michael’s patch.  I’ll rebase this patch after Michael’s 32-bit patch is finalized.
Comment 2 Oliver Hunt 2014-01-16 08:50:30 PST
Comment on attachment 221343 [details]
the patch.

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

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:315
> +    addp 8, sp

prior code didn't increment sp prior to the call, should this be in the CLOOP block?
Comment 3 Mark Lam 2014-01-16 08:53:06 PST
Comment on attachment 221343 [details]
the patch.

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

>> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:315
>> +    addp 8, sp
> 
> prior code didn't increment sp prior to the call, should this be in the CLOOP block?

No, the sp adjustment (before and after the call) is needed for all 32-bit ports, and Michael’s patch will make that change.  I have to make the same change here or the C loop code won’t work.
Comment 4 Geoffrey Garen 2014-01-16 09:25:16 PST
Comment on attachment 221343 [details]
the patch.

r=me
Comment 5 Mark Lam 2014-01-17 23:23:27 PST
Landed in r162248 on the jsCStack branch: <http://trac.webkit.org/r162248>.