Thread: Automatic PG_PRINTF_ATTRIBUTE
pg_config_manual.h has been choosing gnu_printf as the PG_PRINTF_ATTRIBUTE for every MinGW build. That invites a torrent of warnings on pre-gcc-4.4 MinGW compilers, including the compiler on buildfarm member narwhal. I'm increasingly using an affected compiler, because it builds twice as quickly as today's gcc. Let's have "configure" detect whether gcc supports gnu_printf before using it. I gather plain "printf" aliases ms_printf on Windows and gnu_printf elsewhere. Therefore, while the new "configure" test applies to all platforms, non-Windows platforms are disinterested in the outcome today. Suppose gcc introduces aix_printf and has plain "printf" alias it on AIX. PostgreSQL will continue to replace platform printf implementations that depart from our format processing expectations, and our own elog.c code processes errmsg() formats. Therefore, gnu_printf would remain the better global choice even if new archetypes become available.
Attachment
Noah Misch <noah@leadboat.com> writes:
> pg_config_manual.h has been choosing gnu_printf as the PG_PRINTF_ATTRIBUTE for
> every MinGW build. That invites a torrent of warnings on pre-gcc-4.4 MinGW
> compilers, including the compiler on buildfarm member narwhal. I'm
> increasingly using an affected compiler, because it builds twice as quickly as
> today's gcc. Let's have "configure" detect whether gcc supports gnu_printf
> before using it. I gather plain "printf" aliases ms_printf on Windows and
> gnu_printf elsewhere. Therefore, while the new "configure" test applies to
> all platforms, non-Windows platforms are disinterested in the outcome today.
> Suppose gcc introduces aix_printf and has plain "printf" alias it on AIX.
> PostgreSQL will continue to replace platform printf implementations that
> depart from our format processing expectations, and our own elog.c code
> processes errmsg() formats. Therefore, gnu_printf would remain the better
> global choice even if new archetypes become available.
No objection here. I'm not 100% convinced by your argument that we'd not
need to modify the logic in future ... but if we do, we'd still be better
off having it in a configure test than trying to get an #ifdef nest to
do the right thing.
What about the MSVC build path? I guess there we're only targeting
one compiler, so it should be easy.
regards, tom lane
On Fri, Nov 21, 2014 at 01:16:25PM -0500, Tom Lane wrote: > Noah Misch <noah@leadboat.com> writes: > > pg_config_manual.h has been choosing gnu_printf as the PG_PRINTF_ATTRIBUTE for > > every MinGW build. That invites a torrent of warnings on pre-gcc-4.4 MinGW > > compilers, including the compiler on buildfarm member narwhal. I'm > > increasingly using an affected compiler, because it builds twice as quickly as > > today's gcc. Let's have "configure" detect whether gcc supports gnu_printf > > before using it. I gather plain "printf" aliases ms_printf on Windows and > > gnu_printf elsewhere. Therefore, while the new "configure" test applies to > > all platforms, non-Windows platforms are disinterested in the outcome today. > > Suppose gcc introduces aix_printf and has plain "printf" alias it on AIX. > > PostgreSQL will continue to replace platform printf implementations that > > depart from our format processing expectations, and our own elog.c code > > processes errmsg() formats. Therefore, gnu_printf would remain the better > > global choice even if new archetypes become available. > > No objection here. I'm not 100% convinced by your argument that we'd not > need to modify the logic in future ... but if we do, we'd still be better > off having it in a configure test than trying to get an #ifdef nest to > do the right thing. Agreed. The key take-away is that I opted to use gnu_printf on all compilers supporting it, not just on Windows compilers supporting it. > What about the MSVC build path? I guess there we're only targeting > one compiler, so it should be easy. c.h zaps __attribute__(...) under non-__GNUC__ compilers, so we need not touch the MSVC build system.
Hi, On 2014-11-21 03:12:14 -0500, Noah Misch wrote: > pg_config_manual.h has been choosing gnu_printf as the PG_PRINTF_ATTRIBUTE for > every MinGW build. That invites a torrent of warnings on pre-gcc-4.4 MinGW > compilers, including the compiler on buildfarm member narwhal. I'm > increasingly using an affected compiler, because it builds twice as quickly as > today's gcc. No objections to the patch itself, but this seems like quite the odd approach. Sure those old compilers might be a bit faster, but they also report many fewer legitimate warnings and such. A full tree doesn't take that long? A full recompile sess than 40s here and src/backend alone is much faster. ISTM that if it's currently too slow, improving the concurrency of the build a bit more is a wiser path... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-11-21 03:12:14 -0500, Noah Misch wrote:
>> ... I'm
>> increasingly using an affected compiler, because it builds twice as quickly as
>> today's gcc.
> No objections to the patch itself, but this seems like quite the odd
> approach. Sure those old compilers might be a bit faster, but they also
> report many fewer legitimate warnings and such.
> A full tree doesn't take that long? A full recompile sess than 40s here
> and src/backend alone is much faster. ISTM that if it's currently too
> slow, improving the concurrency of the build a bit more is a wiser
> path...
As far as that goes, ccache is a miracle worker. But I don't know
how well it works on Windows :-(
regards, tom lane
On Fri, Nov 21, 2014 at 08:13:17PM -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-11-21 03:12:14 -0500, Noah Misch wrote: > >> ... I'm > >> increasingly using an affected compiler, because it builds twice as quickly as > >> today's gcc. > > > No objections to the patch itself, but this seems like quite the odd > > approach. Sure those old compilers might be a bit faster, but they also > > report many fewer legitimate warnings and such. > > > A full tree doesn't take that long? A full recompile sess than 40s here > > and src/backend alone is much faster. On my Windows development system, it improved a full build from 101s to 58s. That felt substantial, but the risk around warnings is also substantial. I haven't bothered on non-Windows systems. I suspect cross-compiling from GNU/Linux would bring a greater speed improvement, but I have not tried. > > ISTM that if it's currently too > > slow, improving the concurrency of the build a bit more is a wiser > > path... True. Moving from 8 cores to 32 cores gave a disappointing 29% improvement. (You know build time is an obstacle when you start tracking these numbers.) > As far as that goes, ccache is a miracle worker. But I don't know > how well it works on Windows :-( Poorly, in my configuration, unless your cache hit rate is very high. Adding ccache increased the cold build time by 80%.