Bug 234051

Summary: Support WasmAddress in B3 CSE
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
WIP
none
patch
fpizlo: review+
patch for landing
none
patch for landing
none
patch for landing none

Saam Barati
Reported 2021-12-08 18:19:56 PST
...
Attachments
WIP (7.60 KB, patch)
2021-12-09 11:41 PST, Saam Barati 		no flags
DetailsFormatted DiffDiff
WIP (4.12 KB, patch)
2021-12-10 14:59 PST, Saam Barati 		no flags
DetailsFormatted DiffDiff
patch (5.24 KB, patch)
2021-12-16 17:01 PST, Saam Barati 		fpizlo: review+
DetailsFormatted DiffDiff
patch for landing (5.29 KB, patch)
2021-12-16 17:09 PST, Saam Barati 		no flags
DetailsFormatted DiffDiff
patch for landing (5.29 KB, patch)
2021-12-16 17:11 PST, Saam Barati 		no flags
DetailsFormatted DiffDiff
patch for landing (5.31 KB, patch)
2021-12-16 17:12 PST, Saam Barati 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2021-12-09 11:41:32 PST
Created attachment 446579 [details] WIP
Saam Barati
Comment 2 2021-12-10 14:59:24 PST
Created attachment 446822 [details] WIP
Radar WebKit Bug Importer
Comment 3 2021-12-15 18:20:20 PST
<rdar://problem/86552957>
Saam Barati
Comment 4 2021-12-16 17:01:33 PST
Created attachment 447398 [details] patch
Yusuke Suzuki
Comment 5 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.
Yusuke Suzuki
Comment 6 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.
Saam Barati
Comment 7 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.
Saam Barati
Comment 8 2021-12-16 17:09:45 PST
Created attachment 447399 [details] patch for landing
Saam Barati
Comment 9 2021-12-16 17:11:20 PST
Created attachment 447400 [details] patch for landing
Saam Barati
Comment 10 2021-12-16 17:12:14 PST
Created attachment 447401 [details] patch for landing
EWS
Comment 11 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].
Note You need to log in before you can comment on or make changes to this bug.