Bug 162995 - [JSC] Add @throwXXXError bytecode intrinsic
Summary: [JSC] Add @throwXXXError bytecode intrinsic
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-05 22:04 PDT by Yusuke Suzuki
Modified: 2016-10-06 00:46 PDT (History)
7 users (show)

See Also:


Attachments
Patch (79.90 KB, patch)
2016-10-05 23:58 PDT, Yusuke Suzuki
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2016-10-05 22:04:29 PDT
In builtin JS, there are so many `throw @TypeError(".....");` things.
But it spreads so large byte code sequence, effectively enlarges the size of bytecodes, and prevent us from inlining!

[  15] resolve_scope     loc8, loc3, PrivateSymbol.TypeError(@id0), <GlobalVar>, 0, 0x110bdc0a0
[  22] get_from_scope    loc9, loc8, PrivateSymbol.TypeError(@id0), 2049<ThrowIfNotFound|GlobalVar|NotInitialization>, 238692432    predicting None
[  30] mov               loc12, loc9
[  33] mov               loc11, String (atomic) (identifier): Properties can only be defined on Objects., ID: 4(const0)
[  36] construct         loc9, loc9, 2, 18 (this at loc12) status(Could Take Slow Path)    predicting None
[  45] throw             loc9
[  47] ...

Oh, it bloats 32 intructions! We already have great solution. Let's emit op_throw_static_error instead (size is only 3).
Comment 1 Yusuke Suzuki 2016-10-05 22:10:40 PDT
Some motivation: we would like to implement Object.defineProperty in builtin JS and allows it inlined. By doing so, property descriptor object { value: xxx } can get a chance to be allocation sinked in FTL. But it turns out that @TypeError etc. operations easily enlarge the size of byte code sequence and prevent us from inlining.
Comment 2 Yusuke Suzuki 2016-10-05 23:58:13 PDT
Created attachment 290786 [details]
Patch
Comment 3 Saam Barati 2016-10-06 00:40:38 PDT
Comment on attachment 290786 [details]
Patch

Maybe it's worth fixing the builtins in WebCore too? Or maybe in a follow up patch?
Comment 4 Yusuke Suzuki 2016-10-06 00:44:46 PDT
(In reply to comment #3)
> Comment on attachment 290786 [details]
> Patch
> 
> Maybe it's worth fixing the builtins in WebCore too? Or maybe in a follow up
> patch?

Yeah, it's worth fixing in WebCore! I'll upload it separately soon.
Comment 5 Yusuke Suzuki 2016-10-06 00:46:57 PDT
Committed r206853: <http://trac.webkit.org/changeset/206853>