Bug 150655 - we're incorrectly adjusting a stack location with respect to the localsOffset in FTLCompile
Summary: we're incorrectly adjusting a stack location with respect to the localsOffset...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-28 18:42 PDT by Saam Barati
Modified: 2015-10-29 12:15 PDT (History)
11 users (show)

See Also:


Attachments
patch (2.55 KB, patch)
2015-10-28 19:12 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2015-10-28 18:42:56 PDT
we're recomputing this for a *OSRExitDescriptor* for every one of its *OSRExits*. This is having a multiplicative effect on offsets.
I'm amazed this didn't break any of our tests.

I think I introduced this in:
https://bugs.webkit.org/show_bug.cgi?id=149970
Comment 1 Saam Barati 2015-10-28 19:12:06 PDT
Created attachment 264287 [details]
patch
Comment 2 Filip Pizlo 2015-10-28 19:19:21 PDT
Nice catch!
Comment 3 Geoffrey Garen 2015-10-28 19:29:04 PDT
Comment on attachment 264287 [details]
patch

Can haz regression test?
Comment 4 Saam Barati 2015-10-28 22:20:21 PDT
(In reply to comment #3)
> Comment on attachment 264287 [details]
> patch
> 
> Can haz regression test?
I'll try to see if I can come up with one before landing this.
We will definitely have one once FTL try/catch lands but 
it will be nice if I can come up with a small and simple
reproducible program. This might be a bit tricky
to get right. If I can't come up with a small program,
I think coming up with a small program for FTL try/catch
will be easier because of the nature of OSR exits from
from calls and get/put_by_ids.
Comment 5 Saam Barati 2015-10-29 11:28:53 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 264287 [details]
> > patch
> > 
> > Can haz regression test?
> I'll try to see if I can come up with one before landing this.
> We will definitely have one once FTL try/catch lands but 
> it will be nice if I can come up with a small and simple
> reproducible program. This might be a bit tricky
> to get right. If I can't come up with a small program,
> I think coming up with a small program for FTL try/catch
> will be easier because of the nature of OSR exits from
> from calls and get/put_by_ids.
I couldn't get a test case to reproduce this easily.
I'm going to defer until FTL try/catch where we definitely
will have a test case.
Comment 6 Geoffrey Garen 2015-10-29 11:35:40 PDT
Okeedokee.
Comment 7 WebKit Commit Bot 2015-10-29 12:15:37 PDT
Comment on attachment 264287 [details]
patch

Clearing flags on attachment: 264287

Committed r191744: <http://trac.webkit.org/changeset/191744>
Comment 8 WebKit Commit Bot 2015-10-29 12:15:46 PDT
All reviewed patches have been landed.  Closing bug.