Hi, On 2017-09-28 16:21:34 -0400, Tom Lane wrote: > I noticed the following while poking around with perf: > > | fcinfo->isnull = false; > |b5b: movb $0x0,0x1c(%rdx) > | *op->resvalue = > op->d.func.fn_addr(fcinfo); > 0.02 | mov 0x8(%rbx),%rcx > 1.19 | mov %rdx,%rdi > 0.93 | mov %rdx,(%rsp) > | mov %rcx,0x8(%rsp) > 0.01 | callq *0x28(%rbx) > 2.17 | mov 0x8(%rsp),%rcx > | mov %rax,(%rcx) > | *op->resnull = fcinfo->isnull; > 1.18 | mov (%rsp),%rdx > 4.32 | mov 0x10(%rbx),%rax > 0.06 | movzbl 0x1c(%rdx),%edx > 9.14 | mov %dl,(%rax) > > It looks to me like gcc believes it is required to evaluate "op->resvalue" > before invoking the called function, just in case the function somehow has > access to *op and modifies that.
Yea, the compiler probably doesn't have much choice besides assuming that. Might be different if we'd annote function pointers as pure, and used strict aliasing + perhaps some restrict markers, but ... > We could save a pointless register spill > and reload if there were a temporary variable in there, ie > > EEO_CASE(EEOP_FUNCEXPR) > { > FunctionCallInfo fcinfo = op->d.func.fcinfo_data; > + Datum fvalue; > > fcinfo->isnull = false; > - *op->resvalue = op->d.func.fn_addr(fcinfo); > + fvalue = op->d.func.fn_addr(fcinfo); > + *op->resvalue = fvalue; > *op->resnull = fcinfo->isnull; > > EEO_NEXT(); > } > > and likewise in the other FUNCEXPR cases. > > This is on a rather old gcc, haven't checked on bleeding-edge versions. Makes sense. Do you want to make it so, or shall I? I'd personally be somewhat tempted to keep the branches in sync here... Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers