Do we still need PowerPC-specific timestamp_is_current/epoch?

Started by Tom Lanealmost 25 years ago6 messages
#1Tom Lane
tgl@sss.pgh.pa.us

At the end of backend/utils/adt/datetime.c, there is some fairly ugly
code that is conditionally compiled on

#if defined(linux) && defined(__powerpc__)

Do we still need this? The standard versions of TIMESTAMP_IS_CURRENT
and TIMESTAMP_IS_EPOCH appear to work just fine on my Powerbook G3
running Linux 2.2.18 (LinuxPPC 2000 Q4 distro).

I see from the CVS logs that Tatsuo originally introduced this code
on 1997/07/29 (at the time it lived in dt.c and was called
datetime_is_current & datetime_is_epoch). I suppose that it must have
been meant to work around some bug in old versions of gcc for PPC.
But it seems to me to be a net decrease in portability --- it's assuming
that the symbolic constants DBL_MIN and -DBL_MIN will produce particular
bit patterns --- so I'd like to remove it unless someone knows of a
recent Linux/PPC release that still needs it.

regards, tom lane

#2Tatsuo Ishii
t-ishii@sra.co.jp
In reply to: Tom Lane (#1)
Re: Do we still need PowerPC-specific timestamp_is_current/epoch?

At the end of backend/utils/adt/datetime.c, there is some fairly ugly
code that is conditionally compiled on

#if defined(linux) && defined(__powerpc__)

Do we still need this? The standard versions of TIMESTAMP_IS_CURRENT
and TIMESTAMP_IS_EPOCH appear to work just fine on my Powerbook G3
running Linux 2.2.18 (LinuxPPC 2000 Q4 distro).

I see from the CVS logs that Tatsuo originally introduced this code
on 1997/07/29 (at the time it lived in dt.c and was called
datetime_is_current & datetime_is_epoch). I suppose that it must have
been meant to work around some bug in old versions of gcc for PPC.

Yes.

But it seems to me to be a net decrease in portability --- it's assuming
that the symbolic constants DBL_MIN and -DBL_MIN will produce particular
bit patterns --- so I'd like to remove it unless someone knows of a
recent Linux/PPC release that still needs it.

Let me check if my Linux/PPC still needs the workaround.
BTW, what about MkLinux? Anybody tried recent DR5 release?
--
Tatsuo Ishii

#3Tatsuo Ishii
t-ishii@sra.co.jp
In reply to: Tom Lane (#1)
Re: Do we still need PowerPC-specific timestamp_is_current/epoch?

At the end of backend/utils/adt/datetime.c, there is some fairly ugly
code that is conditionally compiled on

#if defined(linux) && defined(__powerpc__)

Do we still need this? The standard versions of TIMESTAMP_IS_CURRENT
and TIMESTAMP_IS_EPOCH appear to work just fine on my Powerbook G3
running Linux 2.2.18 (LinuxPPC 2000 Q4 distro).

I see from the CVS logs that Tatsuo originally introduced this code
on 1997/07/29 (at the time it lived in dt.c and was called
datetime_is_current & datetime_is_epoch). I suppose that it must have
been meant to work around some bug in old versions of gcc for PPC.
But it seems to me to be a net decrease in portability --- it's assuming
that the symbolic constants DBL_MIN and -DBL_MIN will produce particular
bit patterns --- so I'd like to remove it unless someone knows of a
recent Linux/PPC release that still needs it.

After further research, I remembered that we used to have "DB_MIN
check" in configure back to 6.4.2:

AC_MSG_CHECKING(for good DBL_MIN)
AC_TRY_RUN([#include <stdlib.h>
#include <math.h>
#ifdef HAVE_FLOAT_H
# include <float.h>
#endif
main() { double d = DBL_MIN; if (d != DBL_MIN) exit(-1); else exit(0); }],
AC_MSG_RESULT(yes),
[AC_DEFINE(HAVE_DBL_MIN_PROBLEM) AC_MSG_RESULT(no)],
AC_MSG_RESULT(assuming ok on target machine))

I don't know wht it was removed, but I think we'd better to revive the
checking and replace

#if defined(linux) && defined(__powerpc__)

with

#ifdef HAVE_DBL_MIN_PROBLEM

What do you think?
--
Tatsuo Ishii

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tatsuo Ishii (#3)
Re: Do we still need PowerPC-specific timestamp_is_current/epoch?

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

After further research, I remembered that we used to have "DB_MIN
check" in configure back to 6.4.2:
I don't know wht it was removed,

Hmm. Digging in the CVS logs shows that it was removed by Bruce in
configure.in version 1.262, 1999/07/18, with the unedifying log message
"configure cleanup".

A guess is that he took it out because it wasn't being used anywhere.

but I think we'd better to revive the checking and replace
#if defined(linux) && defined(__powerpc__)
with
#ifdef HAVE_DBL_MIN_PROBLEM
What do you think?

I think that is a bad idea, since that code is guaranteed to fail on any
machine where the representation of double is at all different from a
PPC's. (Even if you are willing to assume that the entire world uses
IEEE floats these days, what of endianness?)

We could revive the configure test and do

#if defined(HAVE_DBL_MIN_PROBLEM) && defined(__powerpc__)

However, I really wonder whether there is any point. It may be worth
noting that the original version of the patch read "#if ... defined(PPC)".
It's quite likely that the current test, "... defined(__powerpc__)",
doesn't even fire on the old compiler that the patch is intended for.
If so, this is dead code and has been since release 6.5.

regards, tom lane

#5Tatsuo Ishii
t-ishii@sra.co.jp
In reply to: Tom Lane (#4)
Re: Do we still need PowerPC-specific timestamp_is_current/epoch?

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

After further research, I remembered that we used to have "DB_MIN
check" in configure back to 6.4.2:
I don't know wht it was removed,

Hmm. Digging in the CVS logs shows that it was removed by Bruce in
configure.in version 1.262, 1999/07/18, with the unedifying log message
"configure cleanup".

A guess is that he took it out because it wasn't being used anywhere.

but I think we'd better to revive the checking and replace
#if defined(linux) && defined(__powerpc__)
with
#ifdef HAVE_DBL_MIN_PROBLEM
What do you think?

I think that is a bad idea, since that code is guaranteed to fail on any
machine where the representation of double is at all different from a
PPC's. (Even if you are willing to assume that the entire world uses
IEEE floats these days, what of endianness?)

We could revive the configure test and do

#if defined(HAVE_DBL_MIN_PROBLEM) && defined(__powerpc__)

However, I really wonder whether there is any point. It may be worth
noting that the original version of the patch read "#if ... defined(PPC)".
It's quite likely that the current test, "... defined(__powerpc__)",
doesn't even fire on the old compiler that the patch is intended for.
If so, this is dead code and has been since release 6.5.

Ok, let's remove the code in datetime.c and see anybody would come up
and complain...
--
Tatsuo Ishii

#6Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#4)
Re: Do we still need PowerPC-specific timestamp_is_current/epoch?

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

After further research, I remembered that we used to have "DB_MIN
check" in configure back to 6.4.2:
I don't know wht it was removed,

Hmm. Digging in the CVS logs shows that it was removed by Bruce in
configure.in version 1.262, 1999/07/18, with the unedifying log message
"configure cleanup".

A guess is that he took it out because it wasn't being used anywhere.

Yes, I checked all configure flags and removed the ones not being used.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026