Dear Alexei, Alexei Sheplyakov wrote:
On Sat, Jan 12, 2008 at 12:59:27AM +0100, Richard B. Kreckel wrote:
But I'm still having a hard time understanding how the combination of attributes 'flatten' and 'always_inline' work out.
These attributes make the compiler inline functions more aggressively, so no out of line copies (which cause link failure) produced.
*That* should be explained in source code comments.
Can you, please, do this and also incorporate Bruno's suggestion, then resend the entire patch?
I found out my patch was incomplete.
Which makes me wonder: what's the definition of 'complete', here? Does it work or not? Does it depend on compiler flags like -finline-limit?
In some places non-inline versions of functions was used intstead of inline ones. In principle, they never got inlined anyway (this was exactly the reason of linker errors, and that's why I made the patch in first place). I've tried to replace MAYBE_INLINE the Right Way (TM), however, it turned out to be somewhat difficult. I.e. it's possible to replace either all of MAYBE_INLINEs or none of them. Thus, the correct patch would be even more ugly. Also I failed to convert a couple of (most) obscure instances:
1. src/real/misc/cl_R_eqhashcode.cc
20 #include "cl_I_eqhashcode.cc" 21 #include "cl_SF_eqhashcode.cc" 22 #include "cl_FF_eqhashcode.cc" 23 #include "cl_DF_eqhashcode.cc" 24 #include "cl_LF_eqhashcode.cc"
Why equal_hashcode(const cl_RA&) is not inlined, but other type-specific equal_hashcode functions are?
2. src/rational/misc/cl_RA_eqhashcode.cc
Why equal_hashcode(const cl_I&) is not inlined?
3. include/cln/float.h
23 struct cl_idecoded_float { 24 cl_I mantissa; 25 cl_I exponent; 26 cl_I sign; 27 // Constructor. 28 cl_idecoded_float () {} 29 cl_idecoded_float (const cl_I& m, const cl_I& e, const cl_I& s) : mantissa(m), exponent(e), sign(s) {} 30 };
Why sign is cl_I? It looks like bool would be enough.
Why not cl_I? Remember that small integers are immediate.
4. src/base/string/cl_st_concat2.cc
Why it is necessary to inline cl_make_heap_string? Is it really performance critical? And why CLN needs its own type for strings, what's wrong with std::string?
I've also been pondering the removal of that own string class and replacing it with std::string. But it has never been a priority and the change would be somewhat intrusive. That will have to wait until some time after 1.2.0 now. As to why it is necessary to inline cl_make_heap_string? I can't imagine it is critical. -richy. -- Richard B. Kreckel <http://www.ginac.de/~kreckel/>