← Back to team overview

maria-developers team mailing list archive

Re: bzr commit into MariaDB 5.1, with Maria 1.5:maria branch (knielsen:2698)

 


On 5-May-09, at 2:39 PM, Kristian Nielsen wrote:

Toby Thain <toby@xxxxxxxxxxxxxxxxxxx> writes:
...

+# take only #define options - the others are gcc specific
+DEFS=""
+for F in $debug_cflags ; do
+	expr "$F" : "^-D" && DEFS="$DEFS $F"
+done
+debug_cflags="-O0 -g $DEFS"

Hm. You should not really have to do this ;-). But I agree with
this solution
for now.

Sure, it's ugly. I wish I could think of a better way myself.

Well, the solution would be that SETUP.sh should not put gcc- specific flags in $debug_cflags! But fixing that should not be a blocker for your patch, I think.

Maybe separation of variables for the truly platform-independent flags (like -g, -O, ...) and the non-portable ones that currently assume gcc (-Wxxx) is indicated. Do you have a Windows build system yet?


=== modified file 'storage/innobase/include/univ.i'
--- a/storage/innobase/include/univ.i	2008-03-27 01:40:45 +0000
+++ b/storage/innobase/include/univ.i	2009-05-04 14:54:10 +0000
@@ -58,7 +58,7 @@ of the 32-bit x86 assembler in mutex ope
 /* We only try to do explicit inlining of functions with gcc and
 Microsoft Visual C++ */

-# if !defined(__GNUC__)
+# if !defined(__GNUC__) && !defined(__SUNPRO_C)
 #  undef  UNIV_MUST_NOT_INLINE			/* Remove compiler warning */
 #  define UNIV_MUST_NOT_INLINE
 # endif


What is the purpose of this change?

I am wondering if this change is unintentional (if so it should be
removed)?

Or if this is really what you intended,

This (along with some of the compiler options) are performance
enhancements ... It merely
enables inlining in Sun CC, prevented by the vanilla sources, but
normally done by GCC on other platforms.

Ok, thanks for information.

I think this sounds good. So I suggest to keep it (since you tested that it works, it seems a good change). Just mention it in the commit comment, and fix the comment above the change (which is apparently already wrong since it
mentions Visual C++). I suggest this:

/*
Enable explicit inlining of functions only for compilers known to support
  it.
*/

Will do. Thanks again!

--Toby


 - Kristian.




Follow ups

References