[Webkit-unassigned] [Bug 106739] Fix the atomicIncrement implementation for MIPS GCC

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=106739





--- Comment #28 from Chao-ying Fu <fu at mips.com>  2013-02-12 14:10:26 PST ---
(In reply to comment #27)
> (In reply to comment #19)
> > (In reply to comment #18)
> > > (From update of attachment 184530 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=184530&action=review
> > > 
> > > >> Source/WTF/wtf/Atomics.cpp:90
> > > >> +int64_t __sync_add_and_fetch_8(int64_t volatile* addend, int64_t value)
> > > > 
> > > > __sync_add_and_fetch_8 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
> > > 
> > > So this is a workaround for a bug in the toolchain that is missing this symbol? Maybe that should be explained in a comment and the ChangeLog? What if the toolchain is newer and suddenly provides the symbol? Are version guards missing?
> > 
> > 
> > Dear Simon, 
> > 
> > Thank you for reviewing.
> > 
> > Currently none of the MIPS32 toolchains defines this symbol, that is why we did not provide version guards. Another possibility would be to add this MIPS implementation under a different name, that conforms to the webkit coding style, and select this new symbol using #if CPU(MIPS) in Atomics.h,
> > 
> > This would have the advantage, that the code will still work, if in the future a MIPS toolchain adds this symbol and we can add support for that version of the toolchain with the proper version guards.
> > 
> > Do you like this plan?
> 
> Well, what is the chance of plan C, i.e. fixing this where it should be fixed, upstream in (lib)gcc?
> 
> If we land a workaround in WebKit for this, then IMHO it should include a link to the upstream bug report and it should be temporary.

FYI.  I just opened a gcc bug as follows.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56300
Bug 56300 - Add __sync_fetch_and_add_8 and other 8-byte atomic functions to 32-bit MIPS targets

Thanks!

> 
> _If_ it becomes clear that this does not belong into libgcc (if upstream comes up with a good reason of say rejecting a bug report), then adding the missing functionality under a different name (not using the gcc symbol names) sounds like a good idea. It can then be explained properly in the ChangeLog.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.