Thread: patch to have configure check if CC is intel C compiler

patch to have configure check if CC is intel C compiler

From
Jeremy Drake
Date:
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

Re: patch to have configure check if CC is intel C compiler

From
Peter Eisentraut
Date:
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/

Re: patch to have configure check if CC is intel C compiler

From
Jeremy Drake
Date:
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"

Re: patch to have configure check if CC is intel C compiler

From
Bruce Momjian
Date:
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. +

Re: patch to have configure check if CC is intel C compiler

From
Jeremy Drake
Date:
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

Re: patch to have configure check if CC is intel C compiler

From
Bruce Momjian
Date:
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. +

Re: patch to have configure check if CC is intel C compiler

From
Tom Lane
Date:
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

Re: patch to have configure check if CC is intel C compiler

From
Jeremy Drake
Date:
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".



Re: patch to have configure check if CC is intel C compiler

From
Tom Lane
Date:
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

Re: patch to have configure check if CC is intel C compiler

From
Jeremy Drake
Date:
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.

Re: patch to have configure check if CC is intel C compiler

From
Jeremy Drake
Date:
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

Re: patch to have configure check if CC is intel C compiler

From
Bruce Momjian
Date:
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. +