Re: Patch: Remove gcc dependency in definition of inline functions

From: Marko Kreen <markokr(at)gmail(dot)com>
To: Kurt Harriman <harriman(at)acm(dot)org>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Remove gcc dependency in definition of inline functions
Date: 2009-12-16 14:23:51
Message-ID: e51f66da0912160623n4308967fs899407d355fd2de1@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12/16/09, Kurt Harriman <harriman(at)acm(dot)org> wrote:
> [Please ignore the previous incomplete version of this reply, which I
> sent by mistake. Sorry for the list noise.]
>
> On 12/15/2009 2:09 PM, Marko Kreen wrote:
> >
> > Oh. Ok then. Force-inline seems better fix as we may want to use
> > it for other reasons too (converting big macros).
> >
> > So it seems like a good moment to solve it for gcc too.
>
> Is ordinary inlining not sufficient?
>
> If deluxe inlining is something that might be wanted in the
> future, but isn't needed right now, perhaps we should wait
> until then.

It allows the effects the __forceinline has on MSVC also observe
on GCC. It should not hurt anything, unless we go overboard with
inline usage.

But yeah, it can be omitted. I was just wondering on code structure,
to make it possible to turn forceinline for gcc too.

> > Your worry ii) can be ignored, managing to compile on such
> > compilers is already overachievement.
>
> I think so too. With your opinion added to mine, do we constitute a
> consensus of the pg community? Someone might object that a sample of
> two individuals is insufficiently representative of the whole, but
> away with the pedants: let us not quibble over trifles.

Note that we are not voting here, we are on technical discussion list.
Interested people can voice counter-arguments, if we can overturn them,
we can proceed...

> > The question is now what should we put into configure and what into
> > headers.
>
> PostgreSQL seems mostly to follow the GNU tradition of using
> autoconf rather than thickets of #ifdefs to discover platform
> characteristics such as supported features, variant spellings of
> keywords and identifiers, and the like. This often makes it easier
> to support new platforms, assuming the autoconf mechanism is
> understood.
>
> Autoconf facilitates testing directly for the needed features:
> generally a better approach than hard-coding knowledge of the
> peculiarities of particular compilers, compiler release levels,
> instruction set architectures, etc. For example, instead of
> writing #ifdefs to decide, "if this is gcc, and the version is
> high enough, then __funcname__ should work", it's better to let
> autoconf actually try to compile a program using __funcname__.
> That way PostgreSQL can keep up with the evolution of improved
> and new compilers without continually updating an #ifdef-encoded
> knowledge base of compiler capabilities.

Well, yeah, except on one platform we are mostly bypassing the
autoconf logic and have lot of duplicate logic.

But I'm not opposed putting it into configure, assuming the
final keyword is still called "inline".

> > Simplest would be to have plain AC_C_INLINE in configure
> > and then in header (c.h?):
> >
> > #ifdef _MSC_VER
> > #undef inline
> > #define inline __forceinline
> > #else
> > #ifdef __GNUC__
> > #undef inline
> > #define inline inline __attribute__((always_inline))
> > #endif
> > #endif
> >
> > (Not compile tested :)
>
> This would force every inline. Why do we want that?

The compiler is allowed to uninline a plain "inline" function
if it feels like it. The gcc uses guesstimate instruction
count (before inlining) to decide on that issue.

ATM it's not a problem, but eg. if we want to turn more complex
macros into inlines (heap_getattr()), it may go over limit and get
uninlined.

> For gcc, I think the __attribute__ has to come after the function's
> parameter list, rather than before the return type.

No.

--
marko

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marko Kreen 2009-12-16 14:30:19 Re: Patch: Remove gcc dependency in definition of inline functions
Previous Message Simon Riggs 2009-12-16 14:01:51 Re: An example of bugs for Hot Standby