Bug 163001

Summary: [WebCore][JSC] Use new @throwTypeError and @throwRangeError intrinsics
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, sam, sbarati
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Yusuke Suzuki 2016-10-06 01:15:08 PDT
[WebCore][JSC] Use new @throwTypeError and @throwRangeError intrinsics
Comment 1 Yusuke Suzuki 2016-10-06 01:23:23 PDT
Created attachment 290793 [details]
Patch
Comment 2 Yusuke Suzuki 2016-10-06 10:30:42 PDT
Motivation: https://bugs.webkit.org/show_bug.cgi?id=162995#c1

> 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 3 Yusuke Suzuki 2016-10-06 10:31:50 PDT
And more notes:

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).
[reply] [-] Comment 1
Comment 4 Keith Miller 2016-10-06 10:39:41 PDT
Comment on attachment 290793 [details]
Patch

nice! r=me.
Comment 5 WebKit Commit Bot 2016-10-06 11:02:04 PDT
Comment on attachment 290793 [details]
Patch

Clearing flags on attachment: 290793

Committed r206870: <http://trac.webkit.org/changeset/206870>
Comment 6 WebKit Commit Bot 2016-10-06 11:02:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Saam Barati 2016-10-06 11:45:33 PDT
Maybe we should also remove the TypeError private name to prevent its use?
Comment 8 Saam Barati 2016-10-06 11:45:53 PDT
(In reply to comment #7)
> Maybe we should also remove the TypeError private name to prevent its use?

And the other potential error types.
Comment 9 Yusuke Suzuki 2016-10-06 12:03:35 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > Maybe we should also remove the TypeError private name to prevent its use?
> 
> And the other potential error types.

Nice catch. @throwTypeError can throw any messages.
We can drop it now!
Comment 10 Yusuke Suzuki 2016-10-06 12:44:44 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > Maybe we should also remove the TypeError private name to prevent its use?
> > 
> > And the other potential error types.
> 
> Nice catch. @throwTypeError can throw any messages.
> We can drop it now!

Ah, after investigating it, we figured out @TypeError etc. is still useful when we rejects Promise.
Like, promise.@reject(new @TypeError(""));