From: Tom Lane <tgl@sss.pgh.pa.us>
To: Noah Misch <noah@leadboat.com>
Cc: David Rowley <dgrowleyml@gmail.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Recent failures on buildfarm member hornet
Date: Wed, 07 Oct 2020 17:48:19 -0400
Message-ID: <2397925.1602107299@sss.pgh.pa.us> (raw)
In-Reply-To: <20201007212056.GB1498955@rfd.leadboat.com>
References: <1830491.1601944935@sss.pgh.pa.us>
<20201007014507.GB1422897@rfd.leadboat.com>
<2040113.1602035809@sss.pgh.pa.us>
<20201007021941.GD1422897@rfd.leadboat.com>
<2233910.1602090798@sss.pgh.pa.us>
<CAApHDvovu2O+SLerPK2fCqKKP-q6NWinzACvJkMQ9zZNEiCKJg@mail.gmail.com>
<20201007212056.GB1498955@rfd.leadboat.com>
Noah Misch <noah@leadboat.com> writes:
> On Thu, Oct 08, 2020 at 09:15:12AM +1300, David Rowley wrote:
>> It would be interesting to see gram.s from both cc99baa4~1 and cc99baa4.
> Attached. Both generated like this:
Hm. I'm too lazy to go bone up on PPC64 ABI conventions, but this does
look suspiciously like the compiler is doing what I feared:
GOOD:
lwa r31,4(r27) # fetching list_length(directargs) ?
.line 16295
ori r3,r27,0x0000
bl .list_concat{PR}
ori r0,r0,0x0000
std r3,112(SP)
.line 16295
extsw r3,r31 # ... and passing it to makeInteger
bl .makeInteger{PR}
ori r0,r0,0x0000
BAD:
ori r3,r31,0x0000
bl .list_concat{PR}
ori r0,r0,0x0000
std r3,112(SP)
.line 16288
lwa r3,4(r31) # fetching list_length(directargs) ?
bl .makeInteger{PR}
ori r0,r0,0x0000
(I'm confused about why the line numbers don't match up, since cc99baa4
did not touch gram.y. But whatever.)
I'm tempted to propose the attached small code rearrangement, which
might dissuade the compiler from thinking it can get away with this.
While I concur with your point that an old xlc version might not be
that exciting, there could be other compilers doing the same thing
in the future.
regards, tom lane
Attachments:rearrange-makeOrderedSetArgs.patch (text/x-diff, 905B) downloaddiff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.yindex 0d101d8171..480d168346 100644--- a/src/backend/parser/gram.y+++ b/src/backend/parser/gram.y@@ -16291,7 +16291,7 @@ makeOrderedSetArgs(List *directargs, List *orderedargs,
core_yyscan_t yyscanner)
{
FunctionParameter *lastd = (FunctionParameter *) llast(directargs);
- int ndirectargs;+ Value *ndirectargs;
/* No restriction unless last direct arg is VARIADIC */
if (lastd->mode == FUNC_PARAM_VARIADIC)
@@ -16315,10 +16315,10 @@ makeOrderedSetArgs(List *directargs, List *orderedargs,
}
/* don't merge into the next line, as list_concat changes directargs */
- ndirectargs = list_length(directargs);+ ndirectargs = makeInteger(list_length(directargs));
return list_make2(list_concat(directargs, orderedargs),
- makeInteger(ndirectargs));+ ndirectargs);
}
/* insertSelectOptions()
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, noah@leadboat.com, dgrowleyml@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Recent failures on buildfarm member hornet
In-Reply-To: <2397925.1602107299@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox