Bug 205625

Summary: [JSC] Remove WTF::loadLoadFence from JSFunction::rareData()
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mark.lam: review+

Yusuke Suzuki
Reported 2019-12-28 21:24:36 PST
Since it is not necessary. We are putting storeStoreFence() when putting RareData into this field. And, we never support concurrent access to this field if CPU does not support appropriate memory-ordering restriction (we strongly assume that acquire-fence for data dependent load is not necessary if store-store fence is appropriately emitted, if this guarantee is not met, we can just disable concurrency for this CPU (DEC Alpha)).
Attachments
Patch (4.72 KB, patch)
2019-12-28 22:44 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (4.91 KB, patch)
2019-12-28 22:47 PST, Yusuke Suzuki 		mark.lam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2019-12-28 22:44:59 PST
Created attachment 386481 [details] Patch
Yusuke Suzuki
Comment 2 2019-12-28 22:47:21 PST
Created attachment 386482 [details] Patch
Mark Lam
Comment 3 2019-12-28 23:13:38 PST
Comment on attachment 386482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386482&action=review r=me > Source/JavaScriptCore/runtime/JSFunctionInlines.h:149 > // FIXME: Use JSFunction::rareData() once WTF::loadLoadFence is removed. > // https://bugs.webkit.org/show_bug.cgi?id=205625 Remove this FIXME.
Yusuke Suzuki
Comment 4 2019-12-29 21:36:06 PST
Comment on attachment 386482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386482&action=review Thanks >> Source/JavaScriptCore/runtime/JSFunctionInlines.h:149 >> // https://bugs.webkit.org/show_bug.cgi?id=205625 > > Remove this FIXME. Removed.
Yusuke Suzuki
Comment 5 2019-12-29 21:36:35 PST
Committed r253943: <https://trac.webkit.org/changeset/253943>
Radar WebKit Bug Importer
Comment 6 2019-12-29 21:37:15 PST
<rdar://problem/58236317>
Saam Barati
Comment 7 2019-12-30 12:45:14 PST
Comment on attachment 386482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386482&action=review > Source/JavaScriptCore/runtime/JSFunction.h:-144 > - // The JS thread may be concurrently creating the rare data > - // If we see it, we want to ensure it has been properly created > - WTF::loadLoadFence(); weird. I wonder why we ever had this.
Saam Barati
Comment 8 2019-12-30 12:45:35 PST
(In reply to Saam Barati from comment #7) > Comment on attachment 386482 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=386482&action=review > > > Source/JavaScriptCore/runtime/JSFunction.h:-144 > > - // The JS thread may be concurrently creating the rare data > > - // If we see it, we want to ensure it has been properly created > > - WTF::loadLoadFence(); > > weird. I wonder why we ever had this. The explanation doesn't even make sense, since a loadLoadFence wouldn't help us here
Yusuke Suzuki
Comment 9 2020-01-02 10:36:50 PST
Comment on attachment 386482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386482&action=review >>> Source/JavaScriptCore/runtime/JSFunction.h:-144 >>> - WTF::loadLoadFence(); >> >> weird. I wonder why we ever had this. > > The explanation doesn't even make sense, since a loadLoadFence wouldn't help us here Yeah, this is weird :)
Note You need to log in before you can comment on or make changes to this bug.