Thread: buildfarm - make check failures for leveret on 8.0 and 8.1
the just added new buildfarm member leveret (fedora core5 x86_64) is building with the recently released Intel C-compiler version 9.1. It passes all tests on -HEAD but fails on make check in both REL8_1_STABLE and REL8_0_STABLE. The logs of the later two branches also contain a very large number of annoying(stupid) warnings - those seem to be the result of -HEAD using: CFLAGS=-O2 -mp1 -fno-strict-aliasing -g while the older branches have CFLAGS=-O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g three things to note here: *) why the large difference in the build-flags ? *) the large amount of warnings are probably caused by -Wall *) -HEAD has -mp1 which the intel compiler manual describes as: -mp1 Improve floating-point precision. -mp1 disables fewer optimizations and has less impact on performancethan -mp. could that be the reason why -HEAD passes the float4/float8 regression tests ? Stefan
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> *) why the large difference in the build-flags ?
CVS HEAD configure.in knows about icc and the release branches don't.
I think the changes were only put into HEAD because of lack of testing,
but if we have buildfarm coverage I think it'd be OK to back-port the
configure logic to the prior branches. Any objections?
CVS HEAD configure.in:
# Some versions of GCC support some additional useful warning flags.
# Check whether they are supported, and add them to CFLAGS if so.
if test "$GCC" = yes; then
# ICC pretends to be GCC but it's lying; it doesn't support these options.
# So we have to check if "GCC" is really ICC.
AC_TRY_COMPILE([], [@%:@ifndef __INTEL_COMPILER
choke me
@%:@endif], [ICC=[yes]], [ICC=[no]])
if test "$ICC" = no; then CFLAGS="$CFLAGS -Wall -Wmissing-prototypes -Wpointer-arith -Winline"
PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels]) else # Intel
compilerhas a bug/misoptimization in checking for # division by NAN (NaN == 0), -mp1 fixes it, so add it to the #
CFLAGS. PGAC_PROG_CC_CFLAGS_OPT([-mp1]) fi
# Disable strict-aliasing rules; needed for gcc 3.3+ PGAC_PROG_CC_CFLAGS_OPT([-fno-strict-aliasing])
elif test x"${CC}" = x"xlc"; then # AIX xlc has to have strict aliasing turned off too
PGAC_PROG_CC_CFLAGS_OPT([-qnoansialias])
fi
8.1 equivalent code fragment:
if test "$GCC" = yes; then CFLAGS="$CFLAGS -Wall -Wmissing-prototypes -Wpointer-arith -Winline"
# Some versions of GCC support some additional useful warning flags. # Check whether they are supported, and add them
toCFLAGS if so. PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels])
# Disable strict-aliasing rules; needed for gcc 3.3+ PGAC_PROG_CC_CFLAGS_OPT([-fno-strict-aliasing])
fi
regards, tom lane
On Mon, 7 Aug 2006, Tom Lane wrote: > Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: > > *) why the large difference in the build-flags ? > > CVS HEAD configure.in knows about icc and the release branches don't. > I think the changes were only put into HEAD because of lack of testing, > but if we have buildfarm coverage I think it'd be OK to back-port the > configure logic to the prior branches. Any objections? I sent the original patch. I just sent it for HEAD because a) I could still deal with previous branches by editing Makefile.global by hand after configure, b) I reconfigured older branches seldom enough compared to HEAD that it didn't bother me nearly as much, and c) I figured it would be more readily accepted into HEAD than trying to get it back-ported. Also I was not sure about the acceptance of such things into back branches, since it may be interpreted that supporting a new compiler is a "new feature" and most projects don't like to add new features to old releases. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 3: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faq > -- martin@bdsi.com (no longer valid - where are you now, Martin?)-- from /usr/src/linux/drivers/cdrom/mcd.c
Jeremy Drake wrote: > On Mon, 7 Aug 2006, Tom Lane wrote: > > >> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: >> >>> *) why the large difference in the build-flags ? >>> >> CVS HEAD configure.in knows about icc and the release branches don't. >> I think the changes were only put into HEAD because of lack of testing, >> but if we have buildfarm coverage I think it'd be OK to back-port the >> configure logic to the prior branches. Any objections? >> > > I sent the original patch. I just sent it for HEAD because a) I could > still deal with previous branches by editing Makefile.global by hand after > configure, b) I reconfigured older branches seldom enough compared to > HEAD that it didn't bother me nearly as much, and c) I figured it would be > more readily accepted into HEAD than trying to get it back-ported. Also I > was not sure about the acceptance of such things into back branches, since > it may be interpreted that supporting a new compiler is a "new feature" > and most projects don't like to add new features to old releases. > > In general, yes, but I think if we support a new compiler it would be sensible to support at least the latest stable branch if possible, especially if that mainly means extra configuration rather than changing the code. I'd be happy to see the config changes backported. cheers andrew
On Mon, 7 Aug 2006, Tom Lane wrote: > Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: > > *) why the large difference in the build-flags ? > > CVS HEAD configure.in knows about icc and the release branches don't. > I think the changes were only put into HEAD because of lack of testing, > but if we have buildfarm coverage I think it'd be OK to back-port the > configure logic to the prior branches. Plus if it is backported, I can enable 8.x builds on mongoose (my x86 icc buildfarm box). One reason I like to use icc is that I have found (at least in c++ code) that it had a tendancy to warn me about portability issues more than gcc did. But this was nasty, convoluted, nested-template c++ code where it is much more likely to wander into situations in code that the standard did not define, and compilers had vastly different interpretations, and tracking down these sorts of errors was a matter of trying to glean what exactly the new compiler didn't like from a compiler error which wrapped around the screen at least 3 times. Intel's compiler was one of the most standards-compliant c++ compilers around, which was good about pointing out things that were not strictly compliant, while still accepting the more obscure tricks that the standard did allow. Ah, the good old days ;) > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 3: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faq > -- Neglect of duty does not cease, by repetition, to be neglect of duty. -- Napoleon
On Mon, 7 Aug 2006, Stefan Kaltenbrunner wrote: > the just added new buildfarm member leveret (fedora core5 x86_64) is > building with the recently released Intel C-compiler version 9.1. > It passes all tests on -HEAD but fails on make check in both > REL8_1_STABLE and REL8_0_STABLE. > The logs of the later two branches also contain a very large number of > annoying(stupid) warnings - those seem to be the result of -HEAD using: > > CFLAGS=-O2 -mp1 -fno-strict-aliasing -g > > while the older branches have > > CFLAGS=-O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline > -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g Yep, I submitted the patch for this change a while back because the warnings were driving me nuts, and I got tired of manually adjusting Makefile.global every time I reconfigured. Part of the reason for the extra flags is due to the fact that on Linux boxen, icc masquerades as gcc, so that configure cannot tell the difference. This is intended to make porting easier, since now every configure script in the world does not need to know about icc, it is "close enough" to gcc for most purposes. > > three things to note here: > > *) why the large difference in the build-flags ? Because the removed flags are either not supported in icc, or do something rather stupid (see -Wall for an example of this). > *) the large amount of warnings are probably caused by -Wall That, and -Winline, which to icc means something like "display inlining optimization stage remarks as warnings" which I don't think was the intention of the inclusion of the flag here. -Wall also does things like this. > *) -HEAD has -mp1 which the intel compiler manual describes as: > > -mp1 Improve floating-point precision. -mp1 disables fewer > optimizations and has less impact on performance than > -mp. > > could that be the reason why -HEAD passes the float4/float8 regression > tests ? Exactly. Without -mp1, icc cheats in floating point math, resulting in non-standard behavior. IIRC, this was NaN == 0.0 which according to the standard should not be true, but the code generated by icc without -mp1 meant this was true. I suppose it could be argued that this is a bug or a "feature" resulting in (slightly) faster code at the expense of standards compliance, but in either case, it does not work out here. > > > Stefan > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Have you searched our list archives? > > http://archives.postgresql.org > -- He is not only dull himself, he is the cause of dullness in others. -- Samuel Johnson
Jeremy Drake wrote: > On Mon, 7 Aug 2006, Tom Lane wrote: > >> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: >>> *) why the large difference in the build-flags ? >> CVS HEAD configure.in knows about icc and the release branches don't. >> I think the changes were only put into HEAD because of lack of testing, >> but if we have buildfarm coverage I think it'd be OK to back-port the >> configure logic to the prior branches. > > Plus if it is backported, I can enable 8.x builds on mongoose (my x86 icc > buildfarm box). well with two buildfarm boxes (one 32bit and the other one 64bit) we have pretty good coverage that should allow to backport a rather simple fix like that. Stefan
Jeremy Drake <pgsql@jdrake.com> writes:
> Plus if it is backported, I can enable 8.x builds on mongoose (my x86 icc
> buildfarm box).
Please do --- I've applied the changes in 8.1 and 8.0 branches.
regards, tom lane
Tom Lane wrote: > Jeremy Drake <pgsql@jdrake.com> writes: >> Plus if it is backported, I can enable 8.x builds on mongoose (my x86 icc >> buildfarm box). > > Please do --- I've applied the changes in 8.1 and 8.0 branches. and leveret went green on both 8.0 and 8.1 ... Stefan
Stefan Kaltenbrunner wrote: > Tom Lane wrote: > >> Jeremy Drake <pgsql@jdrake.com> writes: >> >>> Plus if it is backported, I can enable 8.x builds on mongoose (my x86 icc >>> buildfarm box). >>> >> Please do --- I've applied the changes in 8.1 and 8.0 branches. >> > > and leveret went green on both 8.0 and 8.1 ... > > > > Good. Now we need to clean up the huge number of warnings, such as: /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/CORE/proto.h(95): warning #1292: attribute "warn_unused_result" ignored __attribute__warn_unused_result__; ^ and pg_restore.c(332): warning #188: enumerated type mixed with another type AH = OpenArchive(inputFileSpec, opts->format); ^ cheers andrew
Andrew Dunstan wrote: > Stefan Kaltenbrunner wrote: >> Tom Lane wrote: >> >>> Jeremy Drake <pgsql@jdrake.com> writes: >>> >>>> Plus if it is backported, I can enable 8.x builds on mongoose (my >>>> x86 icc >>>> buildfarm box). >>>> >>> Please do --- I've applied the changes in 8.1 and 8.0 branches. >>> >> >> and leveret went green on both 8.0 and 8.1 ... >> >> >> >> > > > Good. Now we need to clean up the huge number of warnings, such as: > > > > /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/CORE/proto.h(95): > warning #1292: attribute "warn_unused_result" ignored > __attribute__warn_unused_result__; > ^ > > and > pg_restore.c(332): warning #188: enumerated type mixed with another type > AH = OpenArchive(inputFileSpec, opts->format); well a large number of those look a bit bogus(annoying) - and icc has ways to disable individual warnings (indicated by the number following the #) like: -wd<L1>[,<L2>,...<LN>] Disable diagnostics L1 through LN. maybe we should use that(ftp://download.intel.com/support/performancetools/c/linux/v9/icc.txt has the full manpage)? Stefan