maria-developers team mailing list archive

[Toby Thain <toby@xxxxxxxxxxxxxxxxxxx>]

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.