(Modified) Patch request for PostgreSQL 7.4 for HP-UX IA-64

ViSolve Open Source Team writes:

1. s_lock.h: modified with inline tas code for the HP-C compiler

What is this line all about?

+#if defined(__HP_aCC) || defined(__HP_cc)

There are no other compilers supported, so this seems redundant.

2. genbki.sh: a one-line change that fixes a string concatenation problem with the
HP-C compiler (specific to included .c files).

You're doing this:

-TMPFILE="$TMPDIR/genbkitmp$$.c"
+TMPFILE="$TMPDIR/genbkitmp$$.h"

I'm afraid this will not fly, because calling the preprocessor is only
portable on .c files. Generally, it's also unwise to rely in this kind of
subtle side effect. We need a general solution.

--
Peter Eisentraut peter_e@gmx.net

ViSolve Open Source Team wrote:

Subject: PostgreSQL Patch: Modified Test-and-set routine for HP-UX (IA-64) specifically for the HP-C compiler

With reference to Tom Lane's response to our previous patch request (at
http://archives.postgresql.org/pgsql-bugs/2003-10/msg00149.php)
this is a modified and more focussed patch request for PostgreSQL for for HP-UX
11i V2 for the Intel Itanium architecture (known to the PostgreSQL code as
IA-64).

Peter mentioned:

What is this line all about?

+#if defined(__HP_aCC) || defined(__HP_cc)

There are no other compilers supported, so this seems redundant.

OK, I will remove this check.

Peter also mentioned:

You're doing this:

-TMPFILE="$TMPDIR/genbkitmp$$.c"
+TMPFILE="$TMPDIR/genbkitmp$$.h"

I'm afraid this will not fly, because calling the preprocessor is only
portable on .c files. Generally, it's also unwise to rely in this kind of
subtle side effect. We need a general solution.

Can you give us a way to test for your preprocessor? Is there some
unique output like --version or -V that we can test and use *.h only in
that case?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce,

Thanks for the comments/questions. A couple of points in response:

1. About the #if defined (_HP_aCC) || defined (_HP_cc)

We agree. That line can go.

2. About the TMPFILE definition change from .c to .h: Unfortunately, the -V option does not work the same way across HP compilers (it works as expected with HP aCC, but not with HP cc).

We have a couple of alternatives here.

a. Using `uname` (to look for HP-UX) and checking to see if $CPP is set to /usr/bin/cc or /opt/aCC/bin/aCC. Not good.
b. Doing nothing. The TMPFILE mod is a trivial change to make with each new version of PostgreSQL that we build for HP-UX. (We had in fact suggested this in our original request email). So if you make no changes to the TMPFILE code, that will be fine too.

thanks
ViSolve OpenSource Team

============================================================================
Return-Path: <pgman@candle.pha.pa.us
Received: (qmail 20935 invoked from network); 30 Nov 2003 04:33:45 -0000
Received: from canlde.pha.pa.us (HELO candle.pha.pa.us) ([207.106.42.251])
(envelope-sender <pgman@candle.pha.pa.us)
by visolve.com (qmail-ldap-1.03) with SMTP
for <opensrc_support_hp@visolve.com; 30 Nov 2003 04:33:45 -0000
Received: (from pgman@localhost)
by candle.pha.pa.us (8.11.6/8.11.6) id hAU4Xcs26486;
Sat, 29 Nov 2003 23:33:38 -0500 (EST)
From: Bruce Momjian <pgman@candle.pha.pa.us
Message-Id: <200311300433.hAU4Xcs26486@candle.pha.pa.us
Subject: Re: [BUGS] (Modified) Patch request for PostgreSQL 7.4 for HP-UX
IA-64
In-Reply-To: <019301c3af01$029c9470$70800d0f@NETMERCURY
To: ViSolve Open Source Team <opensrc_support_hp@visolve.com
Date: Sat, 29 Nov 2003 23:33:38 -0500 (EST)
CC: PostgreSQL-patches <pgsql-patches@postgresql.org,
Peter Eisentraut <peter_e@gmx.net
X-Mailer: ELM [version 2.4ME+ PL108 (25)]
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII

ViSolve Open Source Team wrote:
Subject: PostgreSQL Patch: Modified Test-and-set routine for HP-UX
(IA-64) specifically for the HP-C compiler

With reference to Tom Lane's response to our previous patch request (at
http://archives.postgresql.org/pgsql-bugs/2003-10/msg00149.php)
this is a modified and more focussed patch request for PostgreSQL for
for HP-UX 11i V2 for the Intel Itanium architecture (known to the
PostgreSQL code as IA-64).

Peter mentioned:

What is this line all about?

+#if defined(__HP_aCC) || defined(__HP_cc)

There are no other compilers supported, so this seems redundant.

OK, I will remove this check.

Peter also mentioned:

You're doing this:

-TMPFILE="$TMPDIR/genbkitmp$$.c"
+TMPFILE="$TMPDIR/genbkitmp$$.h"

I'm afraid this will not fly, because calling the preprocessor is only
portable on .c files. Generally, it's also unwise to rely in this kind
of subtle side effect. We need a general solution.

Can you give us a way to test for your preprocessor? Is there some
unique output like --version or -V that we can test and use *.h only
in that case?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
====================================================================================

It just struck me that *maybe* a portable solution would be to put the
awk output inside genbki.sh in a .h file and then #include it in a one
line .c file which is passed to the preprocessor. Would that keep the
(strange) HP compiler happy?

cheers

andrew

ViSolve Open Source Team wrote:

ViSolve Open Source Team wrote:

Bruce,

Thanks for the comments/questions. A couple of points in response:

1. About the #if defined (_HP_aCC) || defined (_HP_cc)

We agree. That line can go.

OK.

2. About the TMPFILE definition change from .c to .h: Unfortunately, the -V option does not work the same way across HP compilers (it works as expected with HP aCC, but not with HP cc).

We have a couple of alternatives here.

a. Using `uname` (to look for HP-UX) and checking to see if $CPP is set to /usr/bin/cc or /opt/aCC/bin/aCC. Not good.
b. Doing nothing. The TMPFILE mod is a trivial change to make with each new version of PostgreSQL that we build for HP-UX. (We had in fact suggested this in our original request email). So if you make no changes to the TMPFILE code, that will be fine too.

Shame that there is no clean solution. Shame there some output from -V
that would identifity it as the HPUX compiler. I am not worried about
your making a custom version, but buy folks who download the community
release and get failures. Well, I guess we will just wait and see how
often it happens.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

"ViSolve Open Source Team" <opensrc_support_hp@visolve.com> writes:

this is a modified and more focussed patch request for PostgreSQL for for H=
P-UX
11i V2 for the Intel Itanium architecture (known to the PostgreSQL code as
IA-64).=20

I looked into applying the TAS part of this patch now that Bruce has
finished revising the TAS code, but I do not believe it is right.
The tas() routine looks reasonable, but not this:

+#define S_UNLOCK(lock) \
+  ( \
+    _Asm_mov_to_ar((_Asm_app_reg)_AREG_CCV, 1UL),\
+      (_Asm_cmpxchg((_Asm_sz)_SZ_W, _SEM_REL, \
+                    (volatile slock_t*)lock, \
+                    0UL, (_Asm_ldhint)_LDHINT_NONE, \
+                    (_UP_MEM_FENCE|_DOWN_MEM_FENCE)) =3D=3D 1) ? 1 : 0 \
+    )

In the first place, S_UNLOCK does not return a value. In the second,
I doubt that you need an xchg instruction at all --- can't you just
assign zero? (If you can't, then the gcc IA64 code is wrong,
because it uses the default version of S_UNLOCK.)

+#define S_LOCK_FREE(lock) ( *(volatile slock_t*)(lock))

This is wrong too, unless you're not using the convention that zero
indicates a free lock.

regards, tom lane

Peter Eisentraut <peter_e@gmx.net> writes:

ViSolve Open Source Team writes:

2. genbki.sh: a one-line change that fixes a string concatenation problem with the
HP-C compiler (specific to included .c files).

You're doing this:

-TMPFILE="$TMPDIR/genbkitmp$$.c"
+TMPFILE="$TMPDIR/genbkitmp$$.h"

I'm afraid this will not fly, because calling the preprocessor is only
portable on .c files. Generally, it's also unwise to rely in this kind of
subtle side effect. We need a general solution.

I've been looking at this problem, and I think that in fact the HP
compilers are exposing a bogus assumption in our code. The reason that
genbki.sh fails on these compilers is that it effectively assumes that
adjacent string literals, such as
"foo" "bar"
will not be concatenated into a single string literal by 'cpp'.
Otherwise, some entries in pg_proc.h get messed up, such as this one:
DATA(insert OID = 1176 ( timestamptz PGNSP PGUID 14 f f t f s 2 1184 "1082 1083" "select cast(($1 + $2) as timestamp with time zone)" - _null_ ));
In future other catalogs might have similar issues.

But since the ANSI C spec requires adjacent string literals to be
concatenated, that step has to happen *someplace* in the compiler.
I cannot fault HP for choosing to implement it in cpp rather than
some later step of compilation. Therefore this is really our fault
not theirs; we are making an unwarranted assumption about the
behavior of cpp.

It strikes me however that there is a really simple solution, which
is to stop invoking cpp at all during genbki.sh. It is not doing
*anything* of value for us ... merely introducing portability hazards.
The generated temp file that is fed to cpp does not contain any
#-sign preprocessor commands, nor any macros to be expanded. When
using gcc, the only difference between cpp's input and output is
the addition of a comment line like this:
# 1 "genbkitmp12116.c"

It appears that there was once provision to do more interesting things
via cpp: the Makefile and genbki shell script contain provisions to
pass additional switches to cpp, which presumably might be used to
insert "-D" switches so that identifiers could be expanded to something
else in the BKI data. But we aren't using that, and have not done so
for a very long time. Instead we expect genbki.sh itself to replace
everything that needs replacing (NAMEDATALEN etc). I see no reason
why we'd abandon that technique in favor of using cpp again.

So I propose that we remove the cpp invocation and associated
infrastructure from genbki.sh. Any objections?

regards, tom lane

Tom Lane wrote:

It appears that there was once provision to do more interesting things
via cpp: the Makefile and genbki shell script contain provisions to
pass additional switches to cpp, which presumably might be used to
insert "-D" switches so that identifiers could be expanded to something
else in the BKI data. But we aren't using that, and have not done so
for a very long time. Instead we expect genbki.sh itself to replace
everything that needs replacing (NAMEDATALEN etc). I see no reason
why we'd abandon that technique in favor of using cpp again.

So I propose that we remove the cpp invocation and associated
infrastructure from genbki.sh. Any objections?

I was wondering why the quote merging was happening only for *.c file in
HP's cpp. And if it is legal for *.c, why not for *.h, and why doesn't
it hit us with other compilers.

Looks like we are not using awk for all the subsitutions. Removing the
cpp call seems like a great idea.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073