Bug 205625 – [JSC] Remove WTF::loadLoadFence from JSFunction::rareData()

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

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)).

[Yusuke Suzuki]

Created attachment 386481 [details]
Patch

[Yusuke Suzuki]

Created attachment 386482 [details]
Patch

[Mark Lam]

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 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]

Committed r253943: <https://trac.webkit.org/changeset/253943>

[Radar WebKit Bug Importer]

<rdar://problem/58236317>

[Saam Barati]

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]

(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 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 :)