Bug 157775 - [JSC] "return this" in a constructor does not need a branch on isObject(this)
Summary: [JSC] "return this" in a constructor does not need a branch on isObject(this)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-16 17:25 PDT by Benjamin Poulain
Modified: 2016-05-16 20:35 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.85 KB, patch)
2016-05-16 17:29 PDT, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch (3.27 KB, patch)
2016-05-16 20:00 PDT, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch for landing (3.28 KB, patch)
2016-05-16 20:06 PDT, Benjamin Poulain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2016-05-16 17:25:47 PDT
[JSC] "return this" in a constructor does not need a branch on isObject(this)
Comment 1 Benjamin Poulain 2016-05-16 17:29:39 PDT
Created attachment 279073 [details]
Patch
Comment 2 Benjamin Poulain 2016-05-16 17:30:06 PDT
                                    Conf#1                    Conf#2                                      

3d-cube                         4.9549+-0.0261     ?      4.9916+-0.0312        ?
3d-morph                        5.0523+-0.0255     ?      5.0780+-0.0304        ?
3d-raytrace                     5.3399+-0.0310            5.3223+-0.0249        
access-binary-trees             2.0966+-0.0164     ^      2.0264+-0.0164        ^ definitely 1.0346x faster
access-fannkuch                 5.8749+-0.0359     ^      5.8014+-0.0308        ^ definitely 1.0127x faster
access-nbody                    2.5241+-0.0162     ?      2.5462+-0.0184        ?
access-nsieve                   2.9690+-0.0152            2.9677+-0.0163        
bitops-3bit-bits-in-byte        1.0807+-0.0084     ?      1.0826+-0.0085        ?
bitops-bits-in-byte             2.7130+-0.0180     ?      2.7253+-0.0201        ?
bitops-bitwise-and              2.0030+-0.0133            1.9902+-0.0108        
bitops-nsieve-bits              3.0466+-0.0198     ?      3.0540+-0.0238        ?
controlflow-recursive           2.3161+-0.0152     ?      2.3349+-0.0179        ?
crypto-aes                      4.4217+-0.0372            4.4038+-0.0318        
crypto-md5                      2.4412+-0.0167            2.4099+-0.0157          might be 1.0130x faster
crypto-sha1                     2.2903+-0.0174     ?      2.2995+-0.0180        ?
date-format-tofte               6.6671+-0.0416            6.6478+-0.0401        
date-format-xparb               4.8312+-0.0372            4.8075+-0.0361        
math-cordic                     2.8409+-0.0129     ?      2.8411+-0.0168        ?
math-partial-sums               4.2179+-0.0366            4.2139+-0.0341        
math-spectral-norm              1.9951+-0.0155            1.9880+-0.0119        
regexp-dna                      6.5850+-0.0456     ?      6.6112+-0.0548        ?
string-base64                   4.2546+-0.0384            4.2394+-0.0342        
string-fasta                    5.6429+-0.0195            5.6271+-0.0216        
string-tagcloud                 8.6104+-0.0471     ?      8.6181+-0.0574        ?
string-unpack-code             18.7322+-0.1295           18.5419+-0.1163          might be 1.0103x faster
string-validate-input           4.1828+-0.0290     ?      4.1885+-0.0282        ?

<arithmetic>                    4.5263+-0.0071            4.5138+-0.0073          might be 1.0028x faster
Comment 3 Saam Barati 2016-05-16 19:33:52 PDT
Comment on attachment 279073 [details]
Patch

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

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3223
> +        bool shouldEmitIsObjectTest = derived || !srcIsThis;

I think that if you're in an ES6 derived constructor, your 'this' value must either be TDZ or an object.
I'm fairly sure then that 'shouldEmitIsObjectTest' can just be !srcIsThis because the TDZ check
above will cover 'this' being TDZ.
Comment 4 Benjamin Poulain 2016-05-16 20:00:08 PDT
Created attachment 279085 [details]
Patch
Comment 5 Saam Barati 2016-05-16 20:01:10 PDT
Comment on attachment 279085 [details]
Patch

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

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3232
>                  emitTDZCheck(&m_thisRegister);

This can be:
if (!srcIsThis)
    emitTDZCheck...
Comment 6 Saam Barati 2016-05-16 20:01:55 PDT
Comment on attachment 279085 [details]
Patch

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

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3232
>>                  emitTDZCheck(&m_thisRegister);
> 
> This can be:
> if (!srcIsThis)
>     emitTDZCheck...

Oh. Never mind
Comment 7 Benjamin Poulain 2016-05-16 20:06:44 PDT
Created attachment 279087 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2016-05-16 20:35:48 PDT
Comment on attachment 279087 [details]
Patch for landing

Clearing flags on attachment: 279087

Committed r200992: <http://trac.webkit.org/changeset/200992>
Comment 9 WebKit Commit Bot 2016-05-16 20:35:53 PDT
All reviewed patches have been landed.  Closing bug.