[Libreoffice] [PUSHED] remove unused declaration - an unlikely fix for armel segfault in regcomp

Michael Meeks michael.meeks at novell.com
Fri Jan 28 07:19:04 PST 2011


On Thu, 2011-01-27 at 20:13 +0200, Jani Monoses wrote:
> What about using GCC atomic builtins like in the attached patch?

	Lovely :-) I would use them exclusively, except for the fact that we
got a substantial speed win on (lets face it, by now rather old
single-processor Intel systems), with our 'if only-one-cpu' hack in the
past, where the interlocked instructions were insanely expensive.
Whether that is still a significant chunk of our userbase, I have no
idea - and whether the sysctl() that gets that setting is even accurate
- (could be a nasty source of errors if 'PROCESSORS' is not
hyper-threading aware) is uncelar.

> If correct (seems so, but I did no test it beyond building it - are
> there tests for this in LibO?)

	I suspect it is rather hard to test this ;-) in many cases these things
are ordered anyway, and finding a threading hazard is hard. However,
your patch looks correct, and appears to correspond with the manual.

>  it has the advantage of covering more than ARM, and for ARM letting
> gcc emit optimal (I hope) code by taking care of differences between
> atomic exchange primitives on ARM pre v6 (SWP) and newer (STREX/LDREX)
> and memory barrier instructions (DMB vs MCR ....).

	Right - so; I also cut out the GCC / PPC special case in favour of this
code - nice work :-)

> IMHO it is better to stick to portable C/C++ if it does not constitute a
> drawback otherwise :)

	Of course - I'd rather like to get the osl_interlock stuff inlined
everywhere it is used; it is something of a madness to call a function,
just to get an interlocked inc / dec :-)

	Perhaps I'll gather enough courage to annoy all the legacy
uni-processor Intel guys in a bit, and put that in-line in a header for
the GCC cases [ if this works out nicely on OSX that is ]

	Thanks,

		Michael.

-- 
 michael.meeks at novell.com  <><, Pseudo Engineer, itinerant idiot



More information about the LibreOffice mailing list