[CLN-list] MAYBE_INLINE patch

[Alexei Sheplyakov]

Hello!

On Thu, Jan 10, 2008 at 11:54:46PM +0100, Richard B. Kreckel wrote:

> Ouch, that patch is ugly as sin.

Unfortunately, this ugliness is imposed by the standard (ANSI C++).

> But if, as a consequence, it will help someone provide DLLs of CLN,
> well, then maybe it should be applied. 

The patch gets rid of some standard violating code. As a consequence,
it helps me to provide DLLs [1].

> However, I'm worried about the maintainability: Can you, please, write a 
> comment explaining what you're doing at some suitable place in the 
> sources?

Non-diff part of my previous email is supposed to explain the patch.
I'm afraid more details will only obfuscate the source. Anyway, here
it is:


CLN wants to inline some functions in some places and not in others.
The reasons to do so are explained here:
http://www.ginac.de/pipermail/ginac-list/2003-November/000433.html

However, MAYBE_INLINE thing is a wrong way. First of all, the `inline'
keyword is only a recommendation, not an order. The compiler is free
to not inline a function (IMHO its decisions are correct more often
than not). And this is what actually happens, see, e.g.

http://www.ginac.de/pipermail/cln-list/2007-October/000317.html

As a side note, the compiler is free to inline functions even if it is
NOT marked as inline (this can be disabled with __attribute__((noinline))
in GNU C/C++).

Secondly, in ANSI C++ a function *must* be either inline in *every*
translation unit, or non-inline in *every* translation unit. Hence,
MAYBE_INLINE is not valid C++ (although it's valid C). The compiler does
not check for violations of this rule (as per standard), so MAYBE_INLINE
kind of works on some platforms (and some versions of the GNU compiler and
some misterious compiler/linker flags. This is not speculation, I used to
get bizzare link errors even on Linux with g++ 3.3 and
CXXFLAGS="-O2 -mcpu=ev6").

The patch renames internal versions of "simple" functions and makes
them (static) inline. Thus, the code does not violate the standard, and
link errors don't happen. However, everything comes at a price. It's
necessary to replace almost every invocation of a function `foo' with
`inline_foo'. That `almost' is the worse thing: in some places `foo' should
NOT be renamed to `inline_foo'. This is ugly, but I don't see any better
*standard-compliant* way to achive the goal.


Does this explantion sound any better? Is it OK to include it in the code?

> (Oh, and don't worry about that cln_1-1 branch: I'll release CLN 1.2.0 
> really soon now.)

The patch was written specifically for CLN version 1.1, since I use that
version of CLN (I'm not going to upgrade anytime soon). I've forward ported
it to the development branch (that take me less than 5 minutes).

> And one last question: Do you have any idea why it's enough converting 
> only these few occurrences?

In all of these cases inlining the function in question is very difficult
or even impossible.

The most obvious is zerop. In cl_I_ring.cc

112 static cl_number_ring_ops<cl_I> I_ops = {
113         cl_I_p,
114         equal,
115         zerop,
116         operator+,
117         operator-,
118         operator-,
119         operator*,
120         square,
121         expt_pos
122 };

Here, the address of zerop is taken, hence, the compiler is forced to emit
out-of-line copy. The same applies to cl_RA_ring.cc

Other cases are more difficult to analyse. Anyway, the compiler is free to
NOT inline functions, and the code[r] should cope with that.

Best regards,
	Alexei

[1] http://theor.jinr.ru/~varg/web/proj/ginac/woe32

-- 
All science is either physics or stamp collecting.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: Digital signature
Url : http://www.cebix.net/pipermail/cln-list/attachments/20080111/ceb913d1/attachment.pgp