Bug 236387 – Use local variable pointer for concurrently load value

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

Use WTF::opaque in some places to ensure access is done only once

[Yusuke Suzuki]

Created attachment 451418 [details]
Patch

[Yusuke Suzuki]

Created attachment 451436 [details]
Patch

[Saam Barati]

Comment on attachment 451436 [details]
Patch

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

I wonder if it's just better to change Weak to do the thing where it doesn't use m_impl in multiple places than to change the code to use opaque everywhere. I don't think llvm in practice will ever emit a load from a field twice when the program says to do it once, even if it can prove that such a heap location hasn't changed

> Source/JavaScriptCore/runtime/WriteBarrier.h:140
> +    explicit operator bool() const { return !!cell(); }
>      
> -    bool operator!() const { return !m_cell; }
> +    bool operator!() const { return !cell(); }

Why do these need to use the opaque version?

[Yusuke Suzuki]

Comment on attachment 451436 [details]
Patch

Hmmm, not sure whether just using `m_impl` once is enough or not, probably it is not enough in terms of the C spec, but yeah, in the current build, it would be OK.
I'll remove WTF::opaque for now.

[Yusuke Suzuki]

Created attachment 451450 [details]
Patch

[Yusuke Suzuki]

Committed r289515 (247046@trunk): <https://commits.webkit.org/247046@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/88725823>