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.
Attachments:
gnu_printf-auto-v1.patchtext/plain; charset=us-asciiDownload
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 2cf74bb..90b56e7 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -51,6 +51,27 @@ fi
])# PGAC_C_INLINE
+# PGAC_C_PRINTF_ARCHETYPE
+# -----------------------
+# Set the format archetype used by gcc to check printf type functions. We
+# prefer "gnu_printf", which includes what glibc uses, such as %m for error
+# strings and %lld for 64 bit long longs. GCC 4.4 introduced it. It makes a
+# dramatic difference on Windows.
+AC_DEFUN([PGAC_PRINTF_ARCHETYPE],
+[AC_CACHE_CHECK([for printf format archetype], pgac_cv_printf_archetype,
+[ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
+[extern int
+pgac_write(int ignore, const char *fmt,...)
+__attribute__((format(gnu_printf, 2, 3)));], [])],
+ [pgac_cv_printf_archetype=gnu_printf],
+ [pgac_cv_printf_archetype=printf])
+ac_c_werror_flag=$ac_save_c_werror_flag])
+AC_DEFINE_UNQUOTED([PG_PRINTF_ATTRIBUTE], [$pgac_cv_printf_archetype],
+ [Define to gnu_printf if compiler supports it, else printf.])
+])# PGAC_PRINTF_ARCHETYPE
+
# PGAC_TYPE_64BIT_INT(TYPE)
# -------------------------
diff --git a/configure b/configure
index 1248b06..463fc27 100755
--- a/configure
+++ b/configure
@@ -10063,6 +10063,42 @@ _ACEOF
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for printf format archetype" >&5
+$as_echo_n "checking for printf format archetype... " >&6; }
+if ${pgac_cv_printf_archetype+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+extern int
+pgac_write(int ignore, const char *fmt,...)
+__attribute__((format(gnu_printf, 2, 3)));
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_printf_archetype=gnu_printf
+else
+ pgac_cv_printf_archetype=printf
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_c_werror_flag=$ac_save_c_werror_flag
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_printf_archetype" >&5
+$as_echo "$pgac_cv_printf_archetype" >&6; }
+
+cat >>confdefs.h <<_ACEOF
+#define PG_PRINTF_ATTRIBUTE $pgac_cv_printf_archetype
+_ACEOF
+
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for flexible array members" >&5
$as_echo_n "checking for flexible array members... " >&6; }
diff --git a/configure.in b/configure.in
index 0a3725f..7f4a330 100644
--- a/configure.in
+++ b/configure.in
@@ -1163,6 +1163,7 @@ fi
m4_defun([AC_PROG_CC_STDC], []) dnl We don't want that.
AC_C_BIGENDIAN
PGAC_C_INLINE
+PGAC_PRINTF_ARCHETYPE
AC_C_FLEXIBLE_ARRAY_MEMBER
PGAC_C_SIGNED
PGAC_C_FUNCNAME_SUPPORT
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 3e78d65..465281c 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -718,6 +718,9 @@
/* PostgreSQL major version as a string */
#undef PG_MAJORVERSION
+/* Define to gnu_printf if compiler supports it, else printf. */
+#undef PG_PRINTF_ATTRIBUTE
+
/* Define to 1 if "static inline" works without unwanted warnings from
compilations where static inline functions are defined but not called. */
#undef PG_USE_INLINE
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 9e25ce0..1e64c10 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -188,22 +188,6 @@
#define MAX_RANDOM_VALUE (0x7FFFFFFF)
/*
- * Set the format style used by gcc to check printf type functions. We really
- * want the "gnu_printf" style set, which includes what glibc uses, such
- * as %m for error strings and %lld for 64 bit long longs. But not all gcc
- * compilers are known to support it, so we just use "printf" which all
- * gcc versions alive are known to support, except on Windows where
- * using "gnu_printf" style makes a dramatic difference. Maybe someday
- * we'll have a configure test for this, if we ever discover use of more
- * variants to be necessary.
- */
-#ifdef WIN32
-#define PG_PRINTF_ATTRIBUTE gnu_printf
-#else
-#define PG_PRINTF_ATTRIBUTE printf
-#endif
-
-/*
* On PPC machines, decide whether to use the mutex hint bit in LWARX
* instructions. Setting the hint bit will slightly improve spinlock
* performance on POWER6 and later machines, but does nothing before that,
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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%.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers