Bug 171079 - virtualThunkFor() needs to materialize its of tagMaskRegister for tail calls.
Summary: virtualThunkFor() needs to materialize its of tagMaskRegister for tail calls.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-20 15:05 PDT by Mark Lam
Modified: 2017-04-20 17:31 PDT (History)
6 users (show)

See Also:


Attachments
proposed patch. (4.30 KB, patch)
2017-04-20 15:52 PDT, Mark Lam
saam: review+
Details | Formatted Diff | Diff
patch for landing. (3.61 KB, patch)
2017-04-20 16:32 PDT, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2017-04-20 15:05:31 PDT
This is because tail calls would restore callee saved registers (and therefore, potentially clobber the tag registers) before jumping to the thunk.

<rdar://problem/31684756>
Comment 1 Mark Lam 2017-04-20 15:52:03 PDT
Created attachment 307654 [details]
proposed patch.
Comment 2 Saam Barati 2017-04-20 16:01:02 PDT
Comment on attachment 307654 [details]
proposed patch.

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

r=me

> Source/JavaScriptCore/jit/AssemblyHelpers.h:429
> +    void emitMaterializeTagMaskInRegister(GPRReg reg)
> +    {
> +        move(MacroAssembler::TrustedImm64(TagTypeNumber), reg);
> +        orPtr(MacroAssembler::TrustedImm32(TagBitTypeOther), reg, reg);
> +    }

Please verify this is less code on X86_64 and ARM64, otherwise, please specialize to those platforms what you do such that we emit less code.
Comment 3 Mark Lam 2017-04-20 16:07:48 PDT
(In reply to Saam Barati from comment #2)
> Comment on attachment 307654 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307654&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/jit/AssemblyHelpers.h:429
> > +    void emitMaterializeTagMaskInRegister(GPRReg reg)
> > +    {
> > +        move(MacroAssembler::TrustedImm64(TagTypeNumber), reg);
> > +        orPtr(MacroAssembler::TrustedImm32(TagBitTypeOther), reg, reg);
> > +    }
> 
> Please verify this is less code on X86_64 and ARM64, otherwise, please
> specialize to those platforms what you do such that we emit less code.

I think it's better to use a single instruction.  I'll switch to doing that.
Comment 4 Mark Lam 2017-04-20 16:32:40 PDT
Created attachment 307658 [details]
patch for landing.
Comment 5 Mark Lam 2017-04-20 17:31:43 PDT
Thanks for the review.  Landed in r215596: <http://trac.webkit.org/r215596>.