Re: Raising our compiler requirements for 9.6

From: Noah Misch <noah(at)leadboat(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Raising our compiler requirements for 9.6
Date: 2015-08-15 16:47:09
Message-ID: 20150815164709.GB2014134@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 11, 2015 at 01:04:48PM +0200, Andres Freund wrote:
> On 2015-08-05 15:46:36 +0200, Andres Freund wrote:
> > Here's a conversion for fastgetattr() and heap_getattr().

> In my opinion this drastically increases readability and thus should be
> applied.

Atomics were a miner's canary for pademelon's trouble with post-de6fd1c
inlining. Expect pademelon to break whenever a frontend-included file gains
an inline function that calls a backend function. Atomics were the initial
examples, but this patch adds more:

$ make -s PROFILE='-O0 -DPG_FORCE_DISABLE_INLINE=1'
pg_resetxlog.o: In function `fastgetattr':
/data/nmisch/src/pg/postgresql/src/bin/pg_resetxlog/../../../src/include/access/htup_details.h:754: undefined reference to `nocachegetattr'
pg_resetxlog.o: In function `heap_getattr':
/data/nmisch/src/pg/postgresql/src/bin/pg_resetxlog/../../../src/include/access/htup_details.h:783: undefined reference to `heap_getsysattr'

The htup_details.h case is trickier in a way, because pg_resetxlog has a
legitimate need for SizeofHeapTupleHeader via TOAST_MAX_CHUNK_SIZE. Expect
this class of problem to recur as we add more inline functions. Our method to
handle these first two instances will set a precedent.

That gave me new respect for STATIC_IF_INLINE. While it does add tedious work
to the task of introducing a new batch of inline functions, the work is
completely mechanical. Anyone can write it; anyone can review it; there's one
correct way to write it. Header surgery like
0001-Don-t-include-low-level-locking-code-from-frontend-c.patch requires
expert design from scratch, and it (trivially) breaks builds for a sample of
non-core code. Let's return to STATIC_IF_INLINE and convert fastgetattr()
therewith. (A possible future line of inquiry is to generate the
STATIC_IF_INLINE transformation at build time, so the handwritten headers can
use C99 inlining directly as though it is always available.)

Thanks,
nm

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-08-15 17:01:59 Re: Raising our compiler requirements for 9.6
Previous Message David Fetter 2015-08-15 16:18:37 Re: [RFC] allow 'semester' in functions EXTRACT, DATE_PART, TO_CHAR and so on.