[CLN-list] MAYBE_INLINE patch

[Richard B. Kreckel]

Dear Alexei,

Alexei Sheplyakov wrote:
>> Ouch, that patch is ugly as sin.
> 
> Unfortunately, this ugliness is imposed by the standard (ANSI C++).

Yes, I'm aware of that.

> 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?

Yes, thanks a lot. But I'm still having a hard time understanding how 
the combination of attributes 'flatten' and 'always_inline' work out. 
Please put in a little comment about that, directly at the point of 
definition in the source code. It would be quite helpful, I think.

Can you, please, do this and also incorporate Bruno's suggestion, then 
resend the entire patch? I'm waiting with the release until this is 
checked in. Thanks a lot!

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

Aha, thank you.

Cheers
   -richy.
-- 
Richard B. Kreckel
<http://www.ginac.de/~kreckel/>