Thread: patch to have configure check if CC is intel C compiler
If configure sees that the compiler specified by $CC looks like gcc (defines __GNUC__), then it puts some extra command line options into the CFLAGS (mostly -W*). The intel C compiler for linux emulates gcc by default, which means it defines that and looks very much like gcc to configure. However, it does not get along with the added -W flags very well. They don't seem to kill it, but some of them give warnings about unsupported command line options and others produce insane amounts of output from the compiler. This patch makes configure check for the __INTEL_COMPILER define (which is the recommended way to detect the intel compiler) before adding the extra CFLAGS if it thinks the compiler is GCC. I am not an autoconf hacker, so I may have done it wrong or something, but it appears to work for me (ICC 9.0.032 on gentoo i686 with latest packages). The patch is against the HEAD but I think it should be similar for other branches (I have been compiling 8.0 and 8.1 for some time with the intel compiler by manually massaging the makefiles after configure ran). -- We the unwilling, led by the ungrateful, are doing the impossible. We've done so much, for so long, with so little, that we are now qualified to do something with nothing.
Attachment
Jeremy Drake wrote: > The intel C compiler for linux emulates gcc > by default, which means it defines that and looks very much like gcc > to configure. However, it does not get along with the added -W flags > very well. They don't seem to kill it, but some of them give > warnings about unsupported command line options and others produce > insane amounts of output from the compiler. Details please. Which options are unsupported and what happens if you use them? -- Peter Eisentraut http://developer.postgresql.org/~petere/
On Sun, 2 Apr 2006, Peter Eisentraut wrote: > Jeremy Drake wrote: > > The intel C compiler for linux emulates gcc > > by default, which means it defines that and looks very much like gcc > > to configure. However, it does not get along with the added -W flags > > very well. They don't seem to kill it, but some of them give > > warnings about unsupported command line options and others produce > > insane amounts of output from the compiler. > > Details please. Which options are unsupported and what happens if you > use them? For an example I grabbed the output from compiling nbtsearch.c using this configure line: CC="icc" CFLAGS="-O3 -ip -parallel -xN" ./configure --with-perl --with-python --with-openssl As you can tell, these make a lot of output. I only include the line(s) with each option which are added when the option is given. So, with the old configure, all of the warnings were concatenated (two instances of the "argument required" warning plus the inlining report and the collection of remark messages). -Wmissing-prototypes and -Wpointer-arith do not appear to make any difference, at least on this file. -Wendif-labels: iccbin: Command line warning: ignoring option '-W'; no argument required -Wdeclaration-after-statement: iccbin: Command line warning: ignoring option '-W'; no argument required -Winline: INLINING REPORT: (_bt_moveright) -> elog_finish(EXTERN) -> elog_start(EXTERN) -> _bt_relandgetbuf(EXTERN) -> ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) INLINING REPORT: (_bt_binsrch) -> ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) INLINING REPORT: (_bt_next) -> _bt_relbuf(EXTERN) -> _bt_checkkeys(EXTERN) -> ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) INLINING REPORT: (_bt_first) -> ARGS_IN_REGS: _bt_endpoint(9) (isz = 475) (sz = 489 (176+313)) -> INLINE: _bt_next(11) (isz = 59) (sz = 73 (27+46)) -> ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) -> _bt_checkkeys(EXTERN) -> _bt_relbuf(EXTERN) -> _bt_relbuf(EXTERN) -> _bt_checkkeys(EXTERN) -> ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) -> ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) -> BufferGetBlockNumber(EXTERN) -> INLINE: _bt_binsrch(15) (isz = 104) (sz = 127 (41+86)) -> ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) -> _bt_freestack(EXTERN) -> ARGS_IN_REGS: _bt_search.(0) (isz = 315) (sz = 335 (113+222)) -> elog_finish(EXTERN) -> elog_start(EXTERN) -> memcpy(EXTERN) -> memcpy(EXTERN) -> ScanKeyEntryInitializeWithInfo(EXTERN) -> index_getprocinfo(EXTERN) -> ScanKeyEntryInitialize(EXTERN) -> get_opclass_proc(EXTERN) -> _bt_preprocess_keys(EXTERN) INLINING REPORT: (_bt_step) -> BufferGetBlockNumber(EXTERN) -> _bt_relbuf(EXTERN) -> _bt_relandgetbuf(EXTERN) -> INLINE: _bt_walk_left(10) (isz = 210) (sz = 222 (77+145)) -> BufferGetBlockNumber(EXTERN) -> _bt_relandgetbuf(EXTERN) -> _bt_relandgetbuf(EXTERN) -> _bt_relandgetbuf(EXTERN) -> elog_start(EXTERN) -> elog_finish(EXTERN) -> elog_start(EXTERN) -> elog_finish(EXTERN) -> _bt_relandgetbuf(EXTERN) -> _bt_relbuf(EXTERN) INLINING REPORT: (_bt_search) -> _bt_relandgetbuf(EXTERN) -> memcpy(EXTERN) -> MemoryContextAlloc(EXTERN) -> BufferGetBlockNumber(EXTERN) -> INLINE: _bt_binsrch(14) (isz = 104) (sz = 127 (41+86)) -> ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) -> INLINE: _bt_moveright(16) (isz = 92) (sz = 110 (37+73)) -> ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) -> _bt_relandgetbuf(EXTERN) -> elog_start(EXTERN) -> elog_finish(EXTERN) -> _bt_getroot(EXTERN) INLINING REPORT: (_bt_compare) -> FunctionCall2(EXTERN) -> nocache_index_getattr(EXTERN) -> nocache_index_getattr(EXTERN) INLINING REPORT: (_bt_endpoint) -> INLINE: _bt_next(12) (isz = 59) (sz = 73 (27+46)) -> ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) -> _bt_checkkeys(EXTERN) -> _bt_relbuf(EXTERN) -> _bt_relbuf(EXTERN) -> _bt_checkkeys(EXTERN) -> ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) -> elog_finish(EXTERN) -> elog_start(EXTERN) -> BufferGetBlockNumber(EXTERN) -> INLINE: _bt_get_endpoint(13) (isz = 199) (sz = 213 (76+137)) -> _bt_gettrueroot(EXTERN) -> _bt_getroot(EXTERN) -> elog_start(EXTERN) -> elog_finish(EXTERN) -> _bt_relandgetbuf(EXTERN) -> elog_start(EXTERN) -> elog_finish(EXTERN) -> _bt_relandgetbuf(EXTERN) INLINING REPORT: (_bt_get_endpoint) -> _bt_relandgetbuf(EXTERN) -> elog_finish(EXTERN) -> elog_start(EXTERN) -> _bt_relandgetbuf(EXTERN) -> elog_finish(EXTERN) -> elog_start(EXTERN) -> _bt_getroot(EXTERN) -> _bt_gettrueroot(EXTERN) -Wall: ../../../../src/include/access/relscan.h(45): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */ ^ ../../../../src/include/access/relscan.h(45): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */ ^ nbtsearch.c(242): remark #810: conversion from "int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits low = P_FIRSTDATAKEY(opaque); ^ nbtsearch.c(243): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) high = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(243): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) high = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(243): remark #810: conversion from "unsigned int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits high = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(273): remark #810: conversion from "int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits OffsetNumber mid = low + ((high - low) / 2); ^ nbtsearch.c(280): remark #810: conversion from "int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits low = mid + 1; ^ nbtsearch.c(369): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) datum = index_getattr(itup, scankey->sk_attno, itupdesc, &isNull); ^ nbtsearch.c(505): remark #279: controlling expression is constant pgstat_count_index_scan(&scan->xs_pgstat_info); ^ nbtsearch.c(868): remark #810: conversion from "BlockNumber={uint32={unsigned int}}" to "uint16={unsigned short}" may lose significant bits ItemPointerSet(current, blkno, offnum); ^ nbtsearch.c(868): remark #810: conversion from "unsigned int" to "uint16={unsigned short}" may lose significant bits ItemPointerSet(current, blkno, offnum); ^ nbtsearch.c(899): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) if (offnum > PageGetMaxOffsetNumber(page)) ^ nbtsearch.c(899): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) if (offnum > PageGetMaxOffsetNumber(page)) ^ nbtsearch.c(963): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(963): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(963): remark #810: conversion from "unsigned int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(992): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(992): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(992): remark #810: conversion from "unsigned int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(993): remark #810: conversion from "int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits offnum = P_FIRSTDATAKEY(opaque); ^ nbtsearch.c(1037): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1037): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1037): remark #810: conversion from "unsigned int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1049): remark #810: conversion from "BlockNumber={uint32={unsigned int}}" to "uint16={unsigned short}" may lose significant bits ItemPointerSet(current, blkno, offnum); ^ nbtsearch.c(1049): remark #810: conversion from "unsigned int" to "uint16={unsigned short}" may lose significant bits ItemPointerSet(current, blkno, offnum); ^ nbtsearch.c(1237): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) offnum = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1237): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) offnum = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1237): remark #810: conversion from "unsigned int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits offnum = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1239): remark #810: conversion from "int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits offnum = P_FIRSTDATAKEY(opaque); ^ nbtsearch.c(1299): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1299): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1299): remark #810: conversion from "unsigned int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits maxoff = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1306): remark #810: conversion from "int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits start = P_FIRSTDATAKEY(opaque); ^ nbtsearch.c(1312): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) start = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1312): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) start = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1312): remark #810: conversion from "unsigned int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits start = PageGetMaxOffsetNumber(page); ^ nbtsearch.c(1314): remark #810: conversion from "int" to "OffsetNumber={uint16={unsigned short}}" may lose significant bits start = P_FIRSTDATAKEY(opaque); ^ nbtsearch.c(1322): remark #810: conversion from "BlockNumber={uint32={unsigned int}}" to "uint16={unsigned short}" may lose significant bits ItemPointerSet(current, blkno, start); ^ nbtsearch.c(1322): remark #810: conversion from "unsigned int" to "uint16={unsigned short}" may lose significant bits ItemPointerSet(current, blkno, start); ^ -- Each team building another component has been using the most recent tested version of the integrated system as a test bed for debugging its piece. Their work will be set back by having that test bed change under them. Of course it must. But the changes need to be quantized. Then each user has periods of productive stability, interrupted by bursts of test-bed change. This seems to be much less disruptive than a constant rippling and trembling. - Frederick Brooks Jr., "The Mythical Man Month"
Comment added and patch applied. Thanks. --------------------------------------------------------------------------- Jeremy Drake wrote: > If configure sees that the compiler specified by $CC looks like gcc > (defines __GNUC__), then it puts some extra command line options into the > CFLAGS (mostly -W*). The intel C compiler for linux emulates gcc by > default, which means it defines that and looks very much like gcc to > configure. However, it does not get along with the added -W flags very > well. They don't seem to kill it, but some of them give warnings about > unsupported command line options and others produce insane amounts of > output from the compiler. > > This patch makes configure check for the __INTEL_COMPILER define (which is > the recommended way to detect the intel compiler) before adding the extra > CFLAGS if it thinks the compiler is GCC. I am not an autoconf hacker, so > I may have done it wrong or something, but it appears to work for me (ICC > 9.0.032 on gentoo i686 with latest packages). > > The patch is against the HEAD but I think it should be similar for other > branches (I have been compiling 8.0 and 8.1 for some time with the intel > compiler by manually massaging the makefiles after configure ran). > > > -- > We the unwilling, led by the ungrateful, are doing the impossible. > We've done so much, for so long, with so little, > that we are now qualified to do something with nothing. Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faq -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Should also warn anyone who tries this that the regression tests for float4 and float8 fail under normal optimization flags. I managed to track it down, and apparently some floating point optimizations (seemingly relating to SSE) cause comparisons involving NaN to give non-standard results. This is worked around in float[48]cmp by explicitly checking isnan. The issue I encountered was when dividing by NaN. float[48]div do a check that if the divisor == 0.0, then a division by zero error is raised. With the non-standard behavior, the comparison NaN == 0 is true, and so dividing by NaN results in a division by zero error rather than the expected result (NaN). The workaround is to give the -mp1 flag to the compiler, which rectifies the behavior. I do not know if this error is important enough to insert the option if the check for the Intel compiler succeeds. The rest of this is an irrelevant but (imho) interesting detailed description of why the code generated by the compiler breaks, and why the -mp1 flag causes it to start working. The exact cause of the nonstandard behavior is an interesting side-effect of the COMISD instruction, in that if the comparison is unordered, all three of the ZF, CF, and PF are set to 1. The optimization results in assembly which looks like (inserted constants instead of registers for readability) comisd 0, NaN je equal ; false equal: ; true The use of the -mp1 flag results in code that checks the parity flag, which when set indicates an unordered result, like this: comisd 0, NaN jp nequal je equal nequal: ; false equal: ; true On Fri, 21 Apr 2006, Bruce Momjian wrote: > > Comment added and patch applied. Thanks. > > --------------------------------------------------------------------------- > > > Jeremy Drake wrote: > > This patch makes configure check for the __INTEL_COMPILER define (which is > > the recommended way to detect the intel compiler) before adding the extra > > CFLAGS if it thinks the compiler is GCC. I am not an autoconf hacker, so > > I may have done it wrong or something, but it appears to work for me (ICC > > 9.0.032 on gentoo i686 with latest packages). -- I tried the clone syscall on me, but it didn't work. -- Mike Neuffer trying to fix a serious time problem
This seems like a compiler bug so I am hoping it will be fixed, or is already fixed in a later release. In fact, I know some users are using the Intel compiler, and we are not hearing reports of regression failures, so I am hoping the release with this bug is not widely used. --------------------------------------------------------------------------- Jeremy Drake wrote: > Should also warn anyone who tries this that the regression tests for > float4 and float8 fail under normal optimization flags. I managed to > track it down, and apparently some floating point optimizations (seemingly > relating to SSE) cause comparisons involving NaN to give non-standard > results. This is worked around in float[48]cmp by explicitly checking > isnan. The issue I encountered was when dividing by NaN. float[48]div do > a check that if the divisor == 0.0, then a division by zero error is > raised. With the non-standard behavior, the comparison NaN == 0 is true, > and so dividing by NaN results in a division by zero error rather than the > expected result (NaN). > > The workaround is to give the -mp1 flag to the compiler, which rectifies > the behavior. > > I do not know if this error is important enough to insert the option if > the check for the Intel compiler succeeds. > > The rest of this is an irrelevant but (imho) interesting detailed > description of why the code generated by the compiler breaks, and why the > -mp1 flag causes it to start working. > > The exact cause of the nonstandard behavior is an interesting side-effect > of the COMISD instruction, in that if the comparison is unordered, all > three of the ZF, CF, and PF are set to 1. The optimization results in > assembly which looks like (inserted constants instead of registers for > readability) > > comisd 0, NaN > je equal > ; false > equal: > ; true > > The use of the -mp1 flag results in code that checks the parity flag, > which when set indicates an unordered result, like this: > > comisd 0, NaN > jp nequal > je equal > nequal: > ; false > equal: > ; true > > > > On Fri, 21 Apr 2006, Bruce Momjian wrote: > > > > > Comment added and patch applied. Thanks. > > > > --------------------------------------------------------------------------- > > > > > > Jeremy Drake wrote: > > > This patch makes configure check for the __INTEL_COMPILER define (which is > > > the recommended way to detect the intel compiler) before adding the extra > > > CFLAGS if it thinks the compiler is GCC. I am not an autoconf hacker, so > > > I may have done it wrong or something, but it appears to work for me (ICC > > > 9.0.032 on gentoo i686 with latest packages). > > > -- > I tried the clone syscall on me, but it didn't work. > -- Mike Neuffer trying to fix a serious time problem > > ---------------------------(end of broadcast)--------------------------- > TIP 9: In versions below 8.0, the planner will ignore your desire to > choose an index scan if your joining column's datatypes do not > match > -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <pgman@candle.pha.pa.us> writes: > This seems like a compiler bug so I am hoping it will be fixed, or is > already fixed in a later release. Yeah. NaN == 0 is just silly ... regards, tom lane
On Fri, 21 Apr 2006, Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > This seems like a compiler bug so I am hoping it will be fixed, or is > > already fixed in a later release. > > Yeah. NaN == 0 is just silly ... From what I can tell from the instruction set docs and test programs, the actual bug/misoptimization is that NaN == anything. Which is even sillier. I actually thought that this was a bug at first, but on further investigation and reading I decided it was a "feature". Because the -mp1 option is supposed to give results closer to ANSI and IEEE standards, specifically with regards to "NaN comparison semantics". Since the only difference in the relevant machine code adding this option is the check for the "unordered" result of the compare, I assume intel made a decision to skip the check, rather than forgetting to check. I managed to find the user's guide online via google, this is the documentation for the -mp1 option: http://ruth.georgetown.edu/comp/intel_cc_90/main_cls/mergedProjects/copts_cls/common_options/option_mp1_lcase.htm Also, as regards -fno-strict-aliasing, the man page says that the default options "Assume aliasing in the program", "Assume aliasing within functions", and "Assume arguments may be aliased". The -fno-strict-aliasing option is not listed in the man page, but it does not complain about it either, and I figured better to include it than to leave it out and risk obscure bugs. It looks like the option for strict aliasing is -ansi-alias which assumes "that the program adheres to the rules defined in the ISO C Standard".
Jeremy Drake <pgsql-patches@jdrake.com> writes: > On Fri, 21 Apr 2006, Tom Lane wrote: >> Yeah. NaN == 0 is just silly ... > From what I can tell from the instruction set docs and test programs, the > actual bug/misoptimization is that NaN == anything. Which is even > sillier. > I actually thought that this was a bug at first, but on further > investigation and reading I decided it was a "feature". Because the -mp1 > option is supposed to give results closer to ANSI and IEEE standards, > specifically with regards to "NaN comparison semantics". Since the only > difference in the relevant machine code adding this option is the check > for the "unordered" result of the compare, I assume intel made a decision > to skip the check, rather than forgetting to check. It's been a very long time since I studied the IEEE arithmetic spec, but my recollection is that comparisons involving NaN are supposed to yield the result "unordered", which is not any of the normal possibilities "less", "equal", or "greater". The problem for C compiler authors is how to wedge that behavior into a language that only admits the three normal possibilities. It sounds like the ICC authors have decided that it's OK to behave randomly, ie, not check for unordered at all, by default. I suspect that whether NaN appears to be "equal" or "unequal" or "less" or "greater" depends tremendously on the details of the assembly code the compiler chances to emit (ie, which arm of the IF it chooses to branch to instead of fall through to). So basically, the default behavior is completely unusable in programs that allow NaN to appear in their computations. Given that we've already got a test for ICC in there as of today, I'd vote for adding -mp1 to CFLAGS if we see it's ICC. BTW, does anyone want to set up a buildfarm member running ICC? regards, tom lane
On Sat, 22 Apr 2006, Tom Lane wrote: > Jeremy Drake <pgsql-patches@jdrake.com> writes: > > On Fri, 21 Apr 2006, Tom Lane wrote: > >> Yeah. NaN == 0 is just silly ... > > > From what I can tell from the instruction set docs and test programs, the > > actual bug/misoptimization is that NaN == anything. Which is even > > sillier. > > It's been a very long time since I studied the IEEE arithmetic spec, but > my recollection is that comparisons involving NaN are supposed to yield > the result "unordered", which is not any of the normal possibilities > "less", "equal", or "greater". The problem for C compiler authors is > how to wedge that behavior into a language that only admits the three > normal possibilities. It sounds like the ICC authors have decided that > it's OK to behave randomly, ie, not check for unordered at all, by > default. I suspect that whether NaN appears to be "equal" or "unequal" > or "less" or "greater" depends tremendously on the details of the > assembly code the compiler chances to emit (ie, which arm of the IF it > chooses to branch to instead of fall through to). So basically, the > default behavior is completely unusable in programs that allow NaN to > appear in their computations. Not quite. The C99 standard specifies that all compares with NaN are false (rather like sql null). But with the code the intel compiler generates, all three flags ZF, CF, and PF are set. This means that all numbers are greater AND equal to NaN unless the parity flag is checked (this indicates an unordered result). But that behavior is decidedly nonstandard, and thus completely unusable in portable code. It would probably be better described as "arbitrary" rather than "random" though. > > Given that we've already got a test for ICC in there as of today, I'd > vote for adding -mp1 to CFLAGS if we see it's ICC. > > BTW, does anyone want to set up a buildfarm member running ICC? Yeah, I was planning to set one up once that patch was committed. I was just getting the stuff together on my box to run it. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend > -- FORTUNE EXPLAINS WHAT JOB REVIEW CATCH PHRASES MEAN: #9 has management potential: Because of his intimate relationship with inanimate objects, the reviewee has been appointed to the critical position of department pencil monitor. inspirational: A true inspiration to others. ("There, but for the grace of God, go I.") adapts to stress: Passes wind, water, or out depending upon the severity of the situation. goal oriented: Continually sets low goals for himself, and usually fails to meet them.
On Sat, 22 Apr 2006, Tom Lane wrote: > Given that we've already got a test for ICC in there as of today, I'd > vote for adding -mp1 to CFLAGS if we see it's ICC. This patch seems to do the trick... Index: configure.in =================================================================== RCS file: /home/jeremyd/local/postgres/cvsuproot/pgsql/configure.in,v retrieving revision 1.460 diff -c -r1.460 configure.in *** configure.in 22 Apr 2006 00:29:41 -0000 1.460 --- configure.in 25 Apr 2006 06:03:12 -0000 *************** *** 263,268 **** --- 263,273 ---- # Check whether they are supported, and add them to CFLAGS if so. PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels]) + else + # Intel compiler has 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+ -- A quarrel is quickly settled when deserted by one party; there is no battle unless there be two. -- Seneca
Patch applied. Thanks. --------------------------------------------------------------------------- Jeremy Drake wrote: > On Sat, 22 Apr 2006, Tom Lane wrote: > > > Given that we've already got a test for ICC in there as of today, I'd > > vote for adding -mp1 to CFLAGS if we see it's ICC. > > This patch seems to do the trick... > > Index: configure.in > =================================================================== > RCS file: /home/jeremyd/local/postgres/cvsuproot/pgsql/configure.in,v > retrieving revision 1.460 > diff -c -r1.460 configure.in > *** configure.in 22 Apr 2006 00:29:41 -0000 1.460 > --- configure.in 25 Apr 2006 06:03:12 -0000 > *************** > *** 263,268 **** > --- 263,273 ---- > # Check whether they are supported, and add them to CFLAGS if so. > PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) > PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels]) > + else > + # Intel compiler has 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+ > > > > > -- > A quarrel is quickly settled when deserted by one party; there is no battle > unless there be two. -- Seneca > > ---------------------------(end of broadcast)--------------------------- > TIP 9: In versions below 8.0, the planner will ignore your desire to > choose an index scan if your joining column's datatypes do not > match > -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +