[webkit-reviews] review granted: [Bug 227757] Fix uses of Dependency::fence with respect to the compiler outsmarting us : [Attachment 433981] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jul 22 10:01:56 PDT 2021
Robin Morisset <rmorisset at apple.com> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 227757: Fix uses of Dependency::fence with respect to the compiler
outsmarting us
https://bugs.webkit.org/show_bug.cgi?id=227757
Attachment 433981: patch
https://bugs.webkit.org/attachment.cgi?id=433981&action=review
--- Comment #11 from Robin Morisset <rmorisset at apple.com> ---
Comment on attachment 433981
--> https://bugs.webkit.org/attachment.cgi?id=433981
patch
View in context: https://bugs.webkit.org/attachment.cgi?id=433981&action=review
r=me
> Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp:-170
> -JSValue SparseArrayEntry::getConcurrently() const
I'm a bit surprised that you had to remove const here.
Could this be fixed by also having a version of loadAndFence that takes a const
T* ?
>> Source/WTF/wtf/Atomics.h:439
>> + static Dependency loadAndFence(T* pointer, T& output)
>
> I could also change the API to be pair based instead of using an out
parameter.
It would be cleaner, but considering that this is going to be on very hot
paths, and my lack of trust in LLVM's abilitiy to optimize a struct return
type, I'm in favor of sticking with the out param.
More information about the webkit-reviews
mailing list