Bug 190264 - [BigInt] Implement ValueBitXor into DFG
Summary: [BigInt] Implement ValueBitXor into DFG
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caio Lima
URL:
Keywords: InRadar
Depends on: 186235
Blocks: 186173
  Show dependency treegraph
 
Reported: 2018-10-03 14:58 PDT by Caio Lima
Modified: 2019-01-20 13:03 PST (History)
8 users (show)

See Also:


Attachments
WIP - Patch (23.51 KB, patch)
2018-10-06 08:27 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
WIP - Patch (24.19 KB, patch)
2018-10-06 11:52 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
WIP - Patch (25.40 KB, patch)
2018-10-09 19:49 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (30.78 KB, patch)
2018-10-21 18:48 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (30.91 KB, patch)
2018-10-21 20:05 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Benchmarks (95.28 KB, text/plain)
2018-10-22 04:55 PDT, Caio Lima
no flags Details
Patch (36.17 KB, patch)
2018-10-31 18:54 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (32.10 KB, patch)
2018-10-31 19:04 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (33.25 KB, patch)
2018-11-01 09:49 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Benchmarks (95.71 KB, text/plain)
2018-11-01 09:50 PDT, Caio Lima
no flags Details
Patch (33.22 KB, patch)
2018-11-08 06:15 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (33.17 KB, patch)
2018-11-12 07:59 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (34.69 KB, patch)
2018-11-13 10:41 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (33.26 KB, patch)
2018-11-14 08:47 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (33.17 KB, patch)
2018-11-18 16:22 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (33.17 KB, patch)
2018-11-26 03:11 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (33.17 KB, patch)
2018-11-26 03:21 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (33.13 KB, patch)
2018-11-28 04:47 PST, Caio Lima
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Lima 2018-10-03 14:58:50 PDT
...
Comment 1 Caio Lima 2018-10-06 08:27:15 PDT
Created attachment 351727 [details]
WIP - Patch
Comment 2 Caio Lima 2018-10-06 11:52:11 PDT
Created attachment 351730 [details]
WIP - Patch

Patch doesn't compile.
Comment 3 Caio Lima 2018-10-09 19:49:24 PDT
Created attachment 351934 [details]
WIP - Patch
Comment 4 Caio Lima 2018-10-21 18:48:51 PDT
Created attachment 352878 [details]
Patch
Comment 5 Caio Lima 2018-10-21 20:05:44 PDT
Created attachment 352880 [details]
Patch
Comment 6 Caio Lima 2018-10-22 04:55:14 PDT
Created attachment 352885 [details]
Benchmarks

Changes are perf neutral.
Comment 7 Caio Lima 2018-10-31 18:54:30 PDT
Created attachment 353569 [details]
Patch
Comment 8 Caio Lima 2018-10-31 19:04:06 PDT
Created attachment 353570 [details]
Patch
Comment 9 Caio Lima 2018-11-01 04:41:53 PDT
Comment on attachment 353570 [details]
Patch

Investigating performance regression
Comment 10 Caio Lima 2018-11-01 09:49:23 PDT
Created attachment 353609 [details]
Patch
Comment 11 Caio Lima 2018-11-01 09:50:56 PDT
Created attachment 353611 [details]
Benchmarks

This patch is perf neutral.
Comment 12 Caio Lima 2018-11-05 06:03:48 PST
Ping Review
Comment 13 Caio Lima 2018-11-08 06:15:55 PST
Created attachment 354235 [details]
Patch
Comment 14 EWS Watchlist 2018-11-08 07:43:09 PST
Comment on attachment 354235 [details]
Patch

Attachment 354235 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/9908802

New failing tests:
stress/ftl-put-by-id-setter-exception.js.dfg-eager-no-cjit-validate
apiTests
Comment 15 Caio Lima 2018-11-12 07:59:56 PST
Created attachment 354559 [details]
Patch
Comment 16 Caio Lima 2018-11-13 10:41:52 PST
Created attachment 354681 [details]
Patch
Comment 17 Caio Lima 2018-11-14 08:47:07 PST
Created attachment 354813 [details]
Patch
Comment 18 Caio Lima 2018-11-18 16:22:45 PST
Created attachment 355243 [details]
Patch
Comment 19 Caio Lima 2018-11-19 16:32:35 PST
Ping review
Comment 20 Caio Lima 2018-11-26 03:11:59 PST
Created attachment 355629 [details]
Patch
Comment 21 Caio Lima 2018-11-26 03:21:59 PST
Created attachment 355630 [details]
Patch
Comment 22 Caio Lima 2018-11-28 04:47:05 PST
Created attachment 355863 [details]
Patch
Comment 23 Yusuke Suzuki 2018-11-29 17:53:17 PST
Comment on attachment 355863 [details]
Patch

r=me
Comment 24 WebKit Commit Bot 2018-11-30 05:25:01 PST
Comment on attachment 355863 [details]
Patch

Clearing flags on attachment: 355863

Committed r238732: <https://trac.webkit.org/changeset/238732>
Comment 25 WebKit Commit Bot 2018-11-30 05:25:03 PST
All reviewed patches have been landed.  Closing bug.
Comment 26 Radar WebKit Bug Importer 2018-11-30 05:26:39 PST
<rdar://problem/46371247>
Comment 27 Saam Barati 2019-01-20 13:03:40 PST
Comment on attachment 355863 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:398
> +    case ValueBitXor:
>      case ValueBitAnd:
> +    case ValueBitOr:

Can we add constant folding for this in the future if the inputs are constant?

> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:167
> +                case ArithBitXor:
> +                    node->setOpAndDefaultFlags(ValueBitXor);
> +                    break;

Did we end up switching this back to be the opposite direction? First emit ValueBitXor, then optimize to ArithBitXor.