Bug 181176 - Remove op_assert and make @assert in builtins a function call so we have DFG/FTL coverage for builtins that use @assert in debug builds
Summary: Remove op_assert and make @assert in builtins a function call so we have DFG/...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-27 12:30 PST by Saam Barati
Modified: 2018-01-02 12:57 PST (History)
16 users (show)

See Also:


Attachments
patch (12.62 KB, patch)
2017-12-27 14:39 PST, Saam Barati
ysuzuki: review+
Details | Formatted Diff | Diff
patch for landing (12.70 KB, patch)
2017-12-28 18:17 PST, Saam Barati
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (2.62 MB, application/zip)
2017-12-28 19:40 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2017-12-27 12:30:45 PST
...
Comment 1 Saam Barati 2017-12-27 14:39:53 PST
Created attachment 330226 [details]
patch
Comment 2 Yusuke Suzuki 2017-12-28 03:14:20 PST
Comment on attachment 330226 [details]
patch

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

r=me

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:875
> +            return generator.emitLoad(nullptr, jsUndefined());

Let’s load value to dst for consistency. And let’s check ignoreResult so that we do not need to emit unnecessary mov.
Comment 3 Saam Barati 2017-12-28 18:17:07 PST
Created attachment 330238 [details]
patch for landing
Comment 4 EWS Watchlist 2017-12-28 19:40:41 PST
Comment on attachment 330238 [details]
patch for landing

Attachment 330238 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5856372

New failing tests:
fast/mediastream/MediaStream-MediaElement-setObject-null.html
Comment 5 EWS Watchlist 2017-12-28 19:40:43 PST
Created attachment 330242 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 6 Saam Barati 2017-12-28 20:54:02 PST
(In reply to Build Bot from comment #4)
> Comment on attachment 330238 [details]
> patch for landing
> 
> Attachment 330238 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.webkit.org/results/5856372
> 
> New failing tests:
> fast/mediastream/MediaStream-MediaElement-setObject-null.html

This is quite surprising. Building locally now to see if I can reproduce it.
Comment 7 Saam Barati 2017-12-28 23:32:00 PST
(In reply to Saam Barati from comment #6)
> (In reply to Build Bot from comment #4)
> > Comment on attachment 330238 [details]
> > patch for landing
> > 
> > Attachment 330238 [details] did not pass mac-wk2-ews (mac-wk2):
> > Output: http://webkit-queues.webkit.org/results/5856372
> > 
> > New failing tests:
> > fast/mediastream/MediaStream-MediaElement-setObject-null.html
> 
> This is quite surprising. Building locally now to see if I can reproduce it.

This fails for me locally without my changes. I'm going to land this patch.
Comment 8 WebKit Commit Bot 2017-12-28 23:52:28 PST
Comment on attachment 330238 [details]
patch for landing

Clearing flags on attachment: 330238

Committed r226310: <https://trac.webkit.org/changeset/226310>
Comment 9 WebKit Commit Bot 2017-12-28 23:52:29 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-01-02 12:57:16 PST
<rdar://problem/36260843>
Comment 11 Radar WebKit Bug Importer 2018-01-02 12:57:18 PST
<rdar://problem/36260842>