Bug 147640 - jsc-tailcall: Align callee save registers names across LLInt and JITs
Summary: jsc-tailcall: Align callee save registers names across LLInt and JITs
Status: RESOLVED DUPLICATE of bug 148658
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks: 147639
  Show dependency treegraph
 
Reported: 2015-08-04 11:46 PDT by Michael Saboff
Modified: 2015-09-14 10:58 PDT (History)
1 user (show)

See Also:


Attachments
Patch (10.70 KB, patch)
2015-08-04 11:57 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-04 11:46:26 PDT
The LLInt and JITs' names and register ids for callee saves are different.  The names and the registers the represent should be the same across all of JavaScriptCore.
Comment 1 Michael Saboff 2015-08-04 11:57:17 PDT
Created attachment 258195 [details]
Patch
Comment 2 Basile Clement 2015-08-04 12:07:40 PDT
Comment on attachment 258195 [details]
Patch

Do we really need the added complexity of architecture-dependent register usage? I think we should keep the LLInt as is, and make sure that we always have regCS1 == tagTypeNumber and regCS2 == tagMask.
Comment 3 Michael Saboff 2015-08-04 12:33:08 PDT
(In reply to comment #2)
> Comment on attachment 258195 [details]
> Patch
> 
> Do we really need the added complexity of architecture-dependent register
> usage? I think we should keep the LLInt as is, and make sure that we always
> have regCS1 == tagTypeNumber and regCS2 == tagMask.

The thinking is uniformity.  The LLInt needs to restore all callee saves and therefore needs names for all callee saves.  Right now the 64 bit LLint code only know about 3 callee saves, but it needs to restore all callee saves as part of catching an exception.

Callee saves are stored in memory in register ID order from low to high addresses.  The new regCSN and csrN alias follow the register ID.  This simplifies coding the store and load order, especially in the LLInt. If regCS1 and regCS2 where kept as the tag value registers and the highest two register ID callee saves, we'd end up with something like the following (X86-64 example):

csr0 / regCS0    rbx
csr1 / regCS1    r14
csr2 / regCS2    r15
csr3 / regCS3    r12
csr4 / regCS4    r13

The store order in memory though would be regCS0, regCS3, regCS4, regCS1, regCS2.

This would have to be explicitly coded in the LLInt.  If we adopt what the patch suggests, for nonFTL tiers regCSN+1 is always stored at a higher address than regCSN, making coding much clearer.

The other result is that csrN is the same as regCSN for all platforms.
Comment 4 Basile Clement 2015-08-04 12:37:34 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 258195 [details]
> > Patch
> > 
> > Do we really need the added complexity of architecture-dependent register
> > usage? I think we should keep the LLInt as is, and make sure that we always
> > have regCS1 == tagTypeNumber and regCS2 == tagMask.
> 
> The thinking is uniformity.  The LLInt needs to restore all callee saves and
> therefore needs names for all callee saves.  Right now the 64 bit LLint code
> only know about 3 callee saves, but it needs to restore all callee saves as
> part of catching an exception.
> 
> Callee saves are stored in memory in register ID order from low to high
> addresses.  The new regCSN and csrN alias follow the register ID.  This
> simplifies coding the store and load order, especially in the LLInt. If
> regCS1 and regCS2 where kept as the tag value registers and the highest two
> register ID callee saves, we'd end up with something like the following
> (X86-64 example):
> 
> csr0 / regCS0    rbx
> csr1 / regCS1    r14
> csr2 / regCS2    r15
> csr3 / regCS3    r12
> csr4 / regCS4    r13
> 
> The store order in memory though would be regCS0, regCS3, regCS4, regCS1,
> regCS2.
> 
> This would have to be explicitly coded in the LLInt.  If we adopt what the
> patch suggests, for nonFTL tiers regCSN+1 is always stored at a higher
> address than regCSN, making coding much clearer.
> 
> The other result is that csrN is the same as regCSN for all platforms.

I see, that makes sense. r=me.
Comment 5 Michael Saboff 2015-08-04 12:47:53 PDT
Committed r187877: <http://trac.webkit.org/changeset/187877>
Comment 6 Basile Clement 2015-08-31 17:59:32 PDT

*** This bug has been marked as a duplicate of bug 148658 ***
Comment 7 Csaba Osztrogonác 2015-09-14 10:58:36 PDT
Comment on attachment 258195 [details]
Patch

Cleared review? from attachment 258195 [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).