Bug 134508 - In needs FTL implementation
Summary: In needs FTL implementation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-01 13:07 PDT by Matthew Mirman
Modified: 2014-07-02 16:47 PDT (History)
5 users (show)

See Also:


Attachments
Added In to the FTL (17.07 KB, patch)
2014-07-01 15:10 PDT, Matthew Mirman
fpizlo: review-
Details | Formatted Diff | Diff
Added In to the FTL (17.54 KB, patch)
2014-07-02 16:11 PDT, Matthew Mirman
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Mirman 2014-07-01 13:07:02 PDT
patch forthcoming.
Comment 1 Matthew Mirman 2014-07-01 15:10:51 PDT
Created attachment 234203 [details]
Added In to the FTL

For ftlopt branch.
Comment 2 Filip Pizlo 2014-07-02 15:43:19 PDT
Comment on attachment 234203 [details]
Added In to the FTL

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

Add a test for the generic In case and change the sizeOfCheckIn.  Otherwise LGTM!

> Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp:70
> +#if CPU(ARM64)
> +    return 44; // TODO: I DON'T YET KNOW WHAT TO PUT HERE
> +#else
> +    return 32; // TODO: I DON'T YET KNOW WHAT TO PUT HERE 
> +#endif

In general, you should put the smallest value that doesn't result in a crash.  In this case it's just the size of a patchable jump, which on x86 is 5 and on ARM is 4.
Comment 3 Matthew Mirman 2014-07-02 16:11:36 PDT
Created attachment 234292 [details]
Added In to the FTL

Fixed size and added that test.  For ftlopt.
Comment 4 Filip Pizlo 2014-07-02 16:18:06 PDT
Comment on attachment 234292 [details]
Added In to the FTL

Nice!  You should land it yourself.  Also, for patches that you intend to land yourself, don't set cq to ?.
Comment 5 Geoffrey Garen 2014-07-02 16:43:41 PDT
Please be sure to run the full set of regression tests on the ftlopt branch before landing this (if you haven't already).
Comment 6 Matthew Mirman 2014-07-02 16:47:57 PDT
Landed in http://trac.webkit.org/changeset/170736