Bug 127086 - CStack: Fix 32-bit C loop LLINT.
Summary: CStack: Fix 32-bit C loop LLINT.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on: 127071
Blocks:
  Show dependency treegraph
 
Reported: 2014-01-16 00:46 PST by Mark Lam
Modified: 2014-01-17 23:23 PST (History)
5 users (show)

See Also:


Attachments
the patch. (10.81 KB, patch)
2014-01-16 01:01 PST, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>.