Thread: (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

"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
timestampwith 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