Обсуждение: BUG #3516: Incomplete #ifdef statement in s_lock.h

The following bug has been logged online:

Bug reference:      3516
Logged by:          Dirk Tilger
Email address:      dirk@miriup.de
PostgreSQL version: 8.2.4
Operating system:   Linux with Intel compiler on ia64
Description:        Incomplete #ifdef statement in s_lock.h
Details:

I have been compiling postgresql 8.0, 8.1 and 8.2.4 with the Intel compiler
in the past successfully. This time something went wrong and although I
can't tell precisely how I triggered it, I have found a fix.

We have an increasingly complex Makefile that compiles our third-party
applications. While playing with the options of the compiler I must have
activated something that triggered the #error on line 809 in file s_lock.h.

I was able to fix the problem by changing line 81 of s_lock.h to:
| #if defined(__GNUC__) || defined(__INTEL_COMPILER) || defined(__ICC)

The icc 9.1.045 manual page says (reformatted):
-----8<-----
__ICC  Value on IA-32 -- 910
       Value on EM64T -- 910
       Value on Itanium Architecture -- NA

Notes -- Assigned value refers to the compiler (e.g., 800 is 8.00).
Supported for legacy reasons. Use __INTEL_COMPILER instead.
-----8<-----

Best regards,
Dirk Tilger

PS: I'm not subscribed to the bugs list, put me in CC for communication.

"Dirk Tilger" <dirk@miriup.de> writes:
> I was able to fix the problem by changing line 81 of s_lock.h to:
> | #if defined(__GNUC__) || defined(__INTEL_COMPILER) || defined(__ICC)

Hm.  Some googling suggests that we should be using only
__INTEL_COMPILER, not both.  configure.in does it that way.

            regards, tom lane

"Dirk Tilger" <dirk@miriup.de> writes:
> Operating system:   Linux with Intel compiler on ia64

> I have been compiling postgresql 8.0, 8.1 and 8.2.4 with the Intel compiler
> in the past successfully. This time something went wrong and although I
> can't tell precisely how I triggered it, I have found a fix.

BTW, what I found in googling suggested that (1) icc never has defined
__ICC on 64-bit machines, and (2) although it does define __GNUC__ by
default, there is a way to turn that off.  So I surmise that your
relevant change was adding a compiler flag that disabled the definition
of __GNUC__.  That doesn't seem to have stopped it from accepting
GNU-style asm directives, though, so I kinda wonder what compiler
behavior does change and what was the point of your flag change.

This suggests that configure.in's check for icc is wrong/incomplete:
it only tests if the compiler was previously determined to be gcc,
ie, it defines __GNUC__.  Now that we know that can be turned off
in icc, it seems like we'd better check all the time.

            regards, tom lane

On Sun, Aug 05, 2007 at 11:20:18AM -0400, Tom Lane wrote:
> "Dirk Tilger" <dirk@miriup.de> writes:
> > Operating system:   Linux with Intel compiler on ia64
>
> > I have been compiling postgresql 8.0, 8.1 and 8.2.4 with the Intel compiler
> > in the past successfully. This time something went wrong and although I
> > can't tell precisely how I triggered it, I have found a fix.
>
> BTW, what I found in googling suggested that (1) icc never has defined
> __ICC on 64-bit machines, and (2) although it does define __GNUC__ by
> default, there is a way to turn that off.  So I surmise that your
> relevant change was adding a compiler flag that disabled the definition
> of __GNUC__.  That doesn't seem to have stopped it from accepting
> GNU-style asm directives, though, so I kinda wonder what compiler
> behavior does change and what was the point of your flag change.

The compiler seems to have been called with:

| icc -mp -no-gcc -mcpu=itanium2 -mtune=itanium2

The manual page about '-no-gcc' says:

-no-gcc
    Do not predefine the __GNUC__, __GNUC_MINOR__, and __GNUC_PATCHLEVEL__
    macros.

FYI: we also apply the attached patch to postgre, so that the
'configure' script would not add the -mp1, when -mp was already
specified. From the manual:

|      -mp    Maintain floating-point precision  (disables  some  opti-
|             mizations).  The  -mp  option  restricts  optimization to
|             maintain declared precision and  to  ensure  that  float-
|             ing-point  arithmetic  conforms  more closely to the ANSI
|             and IEEE standards. For most  programs,  specifying  this
|             option adversely affects performance. If you are not sure
|             whether your application needs this option, try compiling
|             and  running  your  program  both  with and without it to
|             evaluate the effects on both performance and precision.
|
|
|      -mp1   Improve floating-point  precision.  -mp1  disables  fewer
|             optimizations  and  has  less  impact on performance than
|             -mp.

Best regards,
Dirk Tilger.

Dirk Tilger <dirk@miriup.de> writes:
> FYI: we also apply the attached patch to postgre, so that the
> 'configure' script would not add the -mp1, when -mp was already
> specified. From the manual:

There is absolutely no chance that we will accept this patch, as it is
known to break Postgres.

I suggest also that you reconsider the wisdom of using -no-gcc to
compile Postgres, as there are quite a lot of places where we generate
better code if we think it's gcc.  There is no hint in what you showed
that there is any binary compatibility issue between gcc and -no-gcc
modes, so even if you have some other piece of code that fails without
-no-gcc, that's still not a reason to build Postgres that way.

            regards, tom lane