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 tree / graph
 
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
Show Obsolete (16) View All Add an attachment (proposed patch, testcase, etc.)

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.