[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 #27 from Simon Hausmann <hausmann at webkit.org>  2013-02-12 01:33:27 PST ---
(In reply to comment #19)
> (In reply to comment #18)
> > (From update of attachment 184530 [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.

_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.