Re: Recent failures on buildfarm member hornet

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: Recent failures on buildfarm member hornet
Дата
Msg-id 2397925.1602107299@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Recent failures on buildfarm member hornet  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: Recent failures on buildfarm member hornet  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
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

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 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()

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Noah Misch
Дата:
Сообщение: Re: Recent failures on buildfarm member hornet
Следующее
От: Thomas Munro
Дата:
Сообщение: Re: Two fsync related performance issues?