Bug 234051 - Support WasmAddress in B3 CSE
Summary: Support WasmAddress in B3 CSE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-12-08 18:19 PST by Saam Barati
Modified: 2021-12-17 13:07 PST (History)
8 users (show)

See Also:

Attachments
WIP (7.60 KB, patch)
2021-12-09 11:41 PST, Saam Barati 		no flags Details | Formatted Diff | Diff
WIP (4.12 KB, patch)
2021-12-10 14:59 PST, Saam Barati 		no flags Details | Formatted Diff | Diff
patch (5.24 KB, patch)
2021-12-16 17:01 PST, Saam Barati 		fpizlo: review+
Details | Formatted Diff | Diff
patch for landing (5.29 KB, patch)
2021-12-16 17:09 PST, Saam Barati 		no flags Details | Formatted Diff | Diff
patch for landing (5.29 KB, patch)
2021-12-16 17:11 PST, Saam Barati 		no flags Details | Formatted Diff | Diff
patch for landing (5.31 KB, patch)
2021-12-16 17:12 PST, Saam Barati 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) 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 Saam Barati 2021-12-08 18:19:56 PST 
...
Comment 1 Saam Barati 2021-12-09 11:41:32 PST 
Created attachment 446579 [details]
WIP
Comment 2 Saam Barati 2021-12-10 14:59:24 PST 
Created attachment 446822 [details]
WIP
Comment 3 Radar WebKit Bug Importer 2021-12-15 18:20:20 PST 
<rdar://problem/86552957>
Comment 4 Saam Barati 2021-12-16 17:01:33 PST 
Created attachment 447398 [details]
patch
Comment 5 Yusuke Suzuki 2021-12-16 17:04:57 PST 
Comment on attachment 447398 [details]
patch

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

r=me too.

> Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:198
> +                if (WasmAddressValue* wasmAddress = value->as<WasmAddressValue>())
> +                    data.m_wasmAddressesAtTail.add(wasmAddress->child(0), wasmAddress);

Let's avoid adding this if data.writesPinned is already true.

> Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:261
> +        if (WasmAddressValue* wasmAddress = m_value->as<WasmAddressValue>()) {
> +            processWasmAddressValue(wasmAddress);
> +            return;
> +        }

Ditto.
Comment 6 Yusuke Suzuki 2021-12-16 17:06:59 PST 
Comment on attachment 447398 [details]
patch

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

>> Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:198
>> +                    data.m_wasmAddressesAtTail.add(wasmAddress->child(0), wasmAddress);
> 
> Let's avoid adding this if data.writesPinned is already true.

Ah, I misunderstood. This is tracking wasm addresses which are still valid.
Comment 7 Saam Barati 2021-12-16 17:07:30 PST 
Comment on attachment 447398 [details]
patch

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

>> Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:198
>> +                    data.m_wasmAddressesAtTail.add(wasmAddress->child(0), wasmAddress);
> 
> Let's avoid adding this if data.writesPinned is already true.

This is still useful because you could still CSE with values that come *after* writes pinned.
Comment 8 Saam Barati 2021-12-16 17:09:45 PST 
Created attachment 447399 [details]
patch for landing
Comment 9 Saam Barati 2021-12-16 17:11:20 PST 
Created attachment 447400 [details]
patch for landing
Comment 10 Saam Barati 2021-12-16 17:12:14 PST 
Created attachment 447401 [details]
patch for landing
Comment 11 EWS 2021-12-17 13:07:04 PST 
Committed r287203 (245370@main): <https://commits.webkit.org/245370@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447401 [details].