Last night my ancient HP compiler spit up on HEAD:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pademelon&dt=2015-07-01%2001%3A30%3A18
complaining thus:
cpp: "brin_pageops.c", line 626: error 4018: Macro param too large after 
substitution - use -H option.
I was able to revive pademelon by adding a new compiler flag as suggested,
but after looking at what the preprocessor is emitting, I can't say that
I blame it for being unhappy.  This simple-looking line

                Assert(BRIN_IS_REGULAR_PAGE(BufferGetPage(oldbuf)));

is expanding to this:

    do { if (!(((((BrinSpecialSpace *) ( ((void) ((bool) (! (!(((const 
void*)(((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition("!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)", ("FailedAssertion"), "brin_pageops.c", 
626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition("!(( ((void) ((bool) (! 
(!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", 
(\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))", 
("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((oldbuf) < 0) ? 
LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) 
((oldbuf) - 1)) * 8192) ))) != ((void *)0)))) || 
(ExceptionalCondition("!(((const void*)(((Page)( ((void) ((bool) (! (!(( 
((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) !
 >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), 
 >(oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! 
 >(!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
 >(ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= 
 >-NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 
 >0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 
 >0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) 
 >(BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))) != ((void *)0)))", 
 >("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((void) ((bool) (! 
 >(!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! 
 >(!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
 >(ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)", 
 >("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldbuf) != 0 ))) || 
 >(ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
 >(oldbuf) >!
 = -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) 
>= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), 
(oldbuf) != 0 ))", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), 
((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) 
(BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special <= 8192)) || 
(ExceptionalCondition("!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( 
((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", 
(\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || 
(ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers 
&& (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), 
\\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), 
\"brin_pageops.c\", 626), 0)))), 
 ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Bl!
 ock) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special <= 
8192)", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((void) ((bool) (! 
(!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! 
(!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)", 
("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldbuf) != 0 ))) || 
(ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 
0)))), (oldbuf) != 0 ))", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), 
((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) 
(BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special >= 
(__builtin_offsetof (PageHeaderData, pd_linp)))) || 
(ExceptionalCondition("!(((PageHeader) (((Page)( ((void) ((bool) (!!
  (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) 
|| (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= 
-NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), 
(oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) 
<= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= 
-NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 
0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 
0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) 
(BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special >= 
(__builtin_offsetof (PageHeaderData, pd_linp)))", ("FailedAssertion"), 
"brin_pageops.c", 626), 0)))), (char *) ((char *) (((Page)( ((void) ((bool) (! 
(!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffe
 r)", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldb!
 uf) != 0 ))) || (ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), 
\"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))", ("FailedAssertion"), 
"brin_pageops.c", 626), 0)))), ((oldbuf) < 0) ? 
LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) 
((oldbuf) - 1)) * 8192) ))) + ((PageHeader) (((Page)( ((void) ((bool) (! (!(( 
((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)", 
("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldbuf) != 0 ))) || 
(ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 
0)))), (oldbuf) != 0 ))", ("FailedAssertion!
 "), "brin_pageops.c", 626), 0)))), ((oldbuf) < 0) ? 
LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) 
((oldbuf) - 1)) * 8192) ))))->pd_special) ))->vector[(((uintptr_t) ((1)) + ((8) 
- 1)) & ~((uintptr_t) ((8) - 1))) / sizeof(uint16) - 1]) == 0xF093))) 
ExceptionalCondition("!(((((BrinSpecialSpace *) ( ((void) ((bool) (! (!(((const 
void*)(((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), 
\"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || 
(ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers 
&& (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), 
\\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), 
\"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(ol
 dbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) *!
  8192) ))) != ((void *)0)))) || (ExceptionalCondition(\"!(((const 
void*)(((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) 
<= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), 
\\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))) || 
(ExceptionalCondition(\\\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\\\\\"!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)\\\\\\\", 
(\\\\\\\"FailedAssertion\\\\\\\"), \\\\\\\"brin_pageops.c\\\\\\\", 626), 0)))), 
(oldbuf) != 0 ))\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 
0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) 
(BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))) != ((void *)0)))\", 
(\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((void) ((bool) (! 
(!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool!
 ) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", 
(\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || 
(ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers 
&& (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), 
\\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), 
\"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? 
LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) 
((oldbuf) - 1)) * 8192) ))))->pd_special <= 8192)) || 
(ExceptionalCondition(\"!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( 
((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= 
-NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 
0)))), (oldbuf) != 0 ))
 ) || (ExceptionalCondition(\\\"!(( ((void) ((bool) (! (!((oldb!
 uf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\\\\\\\"!((oldbuf) <= NBuffers && (oldbuf) >= 
-NLocBuffer)\\\\\\\", (\\\\\\\"FailedAssertion\\\\\\\"), 
\\\\\\\"brin_pageops.c\\\\\\\", 626), 0)))), (oldbuf) != 0 ))\\\", 
(\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), ((oldbuf) < 0) 
? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) 
((oldbuf) - 1)) * 8192) ))))->pd_special <= 8192)\", (\"FailedAssertion\"), 
\"brin_pageops.c\", 626), 0)))), ((void) ((bool) (! (!(((PageHeader) (((Page)( 
((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) 
>= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) 
>= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), 
(oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) 
<= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= 
-NLocBuffer)\\\",!
  (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 
))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? 
LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) 
((oldbuf) - 1)) * 8192) ))))->pd_special >= (__builtin_offsetof 
(PageHeaderData, pd_linp)))) || (ExceptionalCondition(\"!(((PageHeader) 
(((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers 
&& (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), 
\\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))) || 
(ExceptionalCondition(\\\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\\\\\"!((oldbuf) <= 
NBuffers && (oldbuf) >= -NLocBuffer)\\\\\\\", 
(\\\\\\\"FailedAssertion\\\\\\\"), \\\\\\\"brin_pageops.c\\\\\\\", 626), 0)))), 
(oldbuf) != 0 ))\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626),
  0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - !
 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special 
>= (__builtin_offsetof (PageHeaderData, pd_linp)))\", (\"FailedAssertion\"), 
\"brin_pageops.c\", 626), 0)))), (char *) ((char *) (((Page)( ((void) ((bool) 
(! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) 
|| (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= 
-NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), 
(oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) 
<= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= 
-NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 
0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 
0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) 
(BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))) + ((PageHeader) (((Page)( 
((void) ((bool) (! (!(( ((void) ((bool) (! (!((!
 oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || 
(ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", 
(\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || 
(ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && 
(oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers 
&& (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), 
\\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), 
\"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? 
LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) 
((oldbuf) - 1)) * 8192) ))))->pd_special) ))->vector[(((uintptr_t) ((1)) + ((8) 
- 1)) & ~((uintptr_t) ((8) - 1))) / sizeof(uint16) - 1]) == 0xF093))", 
("FailedAssertion"), "brin_pageops.c", 626); } while (0);

The problem here is that we've got several nested levels of macros that
each feel free to evaluate their arguments multiple times.  Quite aside
from the risk of breaking toolchains, this has got to be inefficient.
A quick look at the generated source code shows five separate occurrences
of the sequence

        testl   %ebp, %ebp
        jns     .L80
        movq    LocalBufferBlockPointers(%rip), %rax
        movq    40(%rsp), %rdx
        movq    (%rax,%rdx), %rax
        jmp     .L81
.L80:
        movq    24(%rsp), %rax
        addq    BufferBlocks(%rip), %rax

corresponding to the BufferGetBlock() macro.  At least gcc seems to
have figured out that it only needed to test BufferIsValid(buffer)
once, but still, this is awful.

And then there's the risk of outright bugs from multiple evaluations
of arguments.

I'm thinking we really ought to mount a campaign to replace some of these
macros with inlined-if-possible functions.

                        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to