RESOLVED FIXED 134508
In needs FTL implementation
https://bugs.webkit.org/show_bug.cgi?id=134508
Summary In needs FTL implementation
Matthew Mirman
Reported 2014-07-01 13:07:02 PDT
patch forthcoming.
Attachments
Added In to the FTL (17.07 KB, patch)
2014-07-01 15:10 PDT, Matthew Mirman
fpizlo: review-
Added In to the FTL (17.54 KB, patch)
2014-07-02 16:11 PDT, Matthew Mirman
fpizlo: review+
Matthew Mirman
Comment 1 2014-07-01 15:10:51 PDT
Created attachment 234203 [details] Added In to the FTL For ftlopt branch.
Filip Pizlo
Comment 2 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.
Matthew Mirman
Comment 3 2014-07-02 16:11:36 PDT
Created attachment 234292 [details] Added In to the FTL Fixed size and added that test. For ftlopt.
Filip Pizlo
Comment 4 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 ?.
Geoffrey Garen
Comment 5 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).
Matthew Mirman
Comment 6 2014-07-02 16:47:57 PDT
Note You need to log in before you can comment on or make changes to this bug.