Bug 148099 - jsc-tailcall: Integrate FTL OSR entry / exit and exceptions handling of callee save registers with other tiers
Summary: jsc-tailcall: Integrate FTL OSR entry / exit and exceptions handling of calle...
Status: RESOLVED DUPLICATE of bug 148666
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on: 148167 148345
Blocks: 146854
  Show dependency treegraph
 
Reported: 2015-08-17 15:09 PDT by Michael Saboff
Modified: 2015-09-14 11:01 PDT (History)
1 user (show)

See Also:


Attachments
Patch (8.37 KB, patch)
2015-08-22 21:21 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
Updated patch including fix for exceptions thrown from the FTL. (9.56 KB, patch)
2015-08-25 15:14 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2015-08-17 15:09:46 PDT
The other tiers now save and restore callee save registers.  The FTL's processing of callee saves for OSR entry, OSR exit and exception unwinding needs to be integrated with the other tiers.
Comment 1 Michael Saboff 2015-08-22 21:21:33 PDT
Created attachment 259733 [details]
Patch
Comment 2 Basile Clement 2015-08-24 13:39:40 PDT
Comment on attachment 259733 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:8
> +        Turned off register preservation thunks for outgoing calls from FTL gerneated code.

gerneated -> generated

> Source/JavaScriptCore/ChangeLog:10
> +        Fxed the FTL OSR exit compiler to restore the callee saves from an FTL funcion to

funcion -> function

> Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp:392
> +        GPRReg regToLoad = baselineRegisterOffset ? GPRInfo::regT0 : reg.gpr();

Why not just always use reg.gpr()?

> Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp:418
> +        if (reg.isLocal() && reg.toLocal() < static_cast<int>(baselineVirtualRegistersForCalleeSaves))

Can this actually happen?
Comment 3 Michael Saboff 2015-08-25 15:12:48 PDT
(In reply to comment #2)
> Comment on attachment 259733 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=259733&action=review
> 
> > Source/JavaScriptCore/ChangeLog:8
> > +        Turned off register preservation thunks for outgoing calls from FTL gerneated code.
> 
> gerneated -> generated

Fixed.

> > Source/JavaScriptCore/ChangeLog:10
> > +        Fxed the FTL OSR exit compiler to restore the callee saves from an FTL funcion to
> 
> funcion -> function

Fixed.

> > Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp:392
> > +        GPRReg regToLoad = baselineRegisterOffset ? GPRInfo::regT0 : reg.gpr();
> 
> Why not just always use reg.gpr()?

Changed.

> > Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp:418
> > +        if (reg.isLocal() && reg.toLocal() < static_cast<int>(baselineVirtualRegistersForCalleeSaves))
> 
> Can this actually happen?

Yes. The FTL sets those values to undefined.
Comment 4 Michael Saboff 2015-08-25 15:14:00 PDT
Created attachment 259887 [details]
Updated patch including fix for exceptions thrown from the FTL.
Comment 5 Basile Clement 2015-08-25 16:05:02 PDT
Comment on attachment 259887 [details]
Updated patch including fix for exceptions thrown from the FTL.

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

r=me

> Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp:417
> +            dataLogF("Got a local that collided with BL callee saves\n");

Revert this.
Comment 6 Michael Saboff 2015-08-26 13:43:48 PDT
Committed r188986: <http://trac.webkit.org/changeset/188986>
Comment 7 Basile Clement 2015-08-31 18:11:03 PDT

*** This bug has been marked as a duplicate of bug 148666 ***
Comment 8 Csaba Osztrogonác 2015-09-14 11:01:10 PDT
Comment on attachment 259887 [details]
Updated patch including fix for exceptions thrown from the FTL.

Cleared review? from attachment 259887 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).