| Summary: | ensureStillAliveHere can take the value in any location | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Keith Miller <keith_miller> | ||||||||
| Component: | New Bugs | Assignee: | Keith Miller <keith_miller> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ews-watchlist, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Keith Miller
2020-04-05 08:56:17 PDT
Created attachment 395509 [details]
Patch
Comment on attachment 395509 [details]
Patch
r=me. Please also fix the variant in JSValue too.
Created attachment 395511 [details]
Patch for landing
(In reply to Mark Lam from comment #2) > Comment on attachment 395509 [details] > Patch > > r=me. Please also fix the variant in JSValue too. Ah, I missed that one. Done. Actually, I think the “clobbers memory” declaration is needed. Otherwise,the compiler can optimize the whole asm statement away. https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html “Note, however, that if the code that follows the asm statement makes no use of any of the output operands, the GCC optimizers may discard the asm statement as unneeded (see Volatile).” I think :memory takes care of this. (In reply to Mark Lam from comment #5) > Actually, I think the “clobbers memory” declaration is needed. > Otherwise,the compiler can optimize the whole asm statement away. > > https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html > “Note, however, that if the code that follows the asm statement makes no use > of any of the output operands, the GCC optimizers may discard the asm > statement as unneeded (see Volatile).” > > I think :memory takes care of this. That's what the volatile does. https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile GCC’s optimizers sometimes discard asm statements if they determine there is no need for the output variables. Also, the optimizers may move code out of loops if they believe that the code will always return the same result (i.e. none of its input values change between calls). Using the volatile qualifier disables these optimizations. Comment on attachment 395511 [details] Patch for landing Nevermind. https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile “ Using the volatile qualifier disables these optimizations. asm statements that have no output operands, including asm goto statements, are implicitly volatile.” Patch 395511 does not build Created attachment 395516 [details]
Patch for landing
Committed r259554: <https://trac.webkit.org/changeset/259554> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395516 [details]. I think this is not correct. This needs to be a compiler-fence and "memory" is used to make it so.
e.g. WTF::compilerFence.
241 // Just a compiler fence. Has no effect on the hardware, but tells the compiler
242 // not to move things around this call. Should not affect the compiler's ability
243 // to do things like register allocation and code motion over pure operations.
244 inline void compilerFence()
245 {
246 #if OS(WINDOWS) && !COMPILER(GCC_COMPATIBLE)
247 _ReadWriteBarrier();
248 #else
249 asm volatile("" ::: "memory");
250 #endif
251 }
Committed r259558: <https://trac.webkit.org/changeset/259558> |