| Summary: | Use local variable pointer for concurrently load value | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||
| Component: | New Bugs | Assignee: | 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
Yusuke Suzuki
2022-02-09 11:28:35 PST
Created attachment 451418 [details]
Patch
Created attachment 451436 [details]
Patch
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? 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.
Created attachment 451450 [details]
Patch
Committed r289515 (247046@trunk): <https://commits.webkit.org/247046@trunk> |