Thread: Alpha test
Seems we have to test for __alpha and __alpha_. This applied patch makes that consistent. -- 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 Index: src/backend/main/main.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/main/main.c,v retrieving revision 1.66 diff -c -c -r1.66 main.c *** src/backend/main/main.c 29 Nov 2003 19:51:49 -0000 1.66 --- src/backend/main/main.c 22 Dec 2003 23:35:40 -0000 *************** *** 23,29 **** #include <pwd.h> #include <unistd.h> ! #if defined(__alpha) && defined(__osf__) #include <sys/sysinfo.h> #include "machine/hal_sysinfo.h" #define ASSEMBLER --- 23,29 ---- #include <pwd.h> #include <unistd.h> ! #if (defined(__alpha) || defined(__alpha__)) && defined(__osf__) #include <sys/sysinfo.h> #include "machine/hal_sysinfo.h" #define ASSEMBLER *************** *** 63,76 **** * without help. Avoid adding more here, if you can. */ ! #if defined(__alpha) #ifdef NOFIXADE int buffer[] = {SSIN_UACPROC, UAC_SIGBUS}; #endif /* NOFIXADE */ #ifdef NOPRINTADE int buffer[] = {SSIN_UACPROC, UAC_NOPRINT}; #endif /* NOPRINTADE */ ! #endif /* __alpha */ #if defined(NOFIXADE) || defined(NOPRINTADE) --- 63,76 ---- * without help. Avoid adding more here, if you can. */ ! #if defined(__alpha) || defined(__alpha__) #ifdef NOFIXADE int buffer[] = {SSIN_UACPROC, UAC_SIGBUS}; #endif /* NOFIXADE */ #ifdef NOPRINTADE int buffer[] = {SSIN_UACPROC, UAC_NOPRINT}; #endif /* NOPRINTADE */ ! #endif /* __alpha || __alpha__ */ #if defined(NOFIXADE) || defined(NOPRINTADE) *************** *** 78,84 **** syscall(SYS_sysmips, MIPS_FIXADE, 0, NULL, NULL, NULL); #endif ! #if defined(__alpha) if (setsysinfo(SSI_NVPAIRS, buffer, 1, (caddr_t) NULL, (unsigned long) NULL) < 0) fprintf(stderr, gettext("%s: setsysinfo failed: %s\n"), --- 78,84 ---- syscall(SYS_sysmips, MIPS_FIXADE, 0, NULL, NULL, NULL); #endif ! #if defined(__alpha) || defined(__alpha__) if (setsysinfo(SSI_NVPAIRS, buffer, 1, (caddr_t) NULL, (unsigned long) NULL) < 0) fprintf(stderr, gettext("%s: setsysinfo failed: %s\n"), Index: src/include/storage/s_lock.h =================================================================== RCS file: /cvsroot/pgsql-server/src/include/storage/s_lock.h,v retrieving revision 1.117 diff -c -c -r1.117 s_lock.h *** src/include/storage/s_lock.h 29 Nov 2003 22:41:13 -0000 1.117 --- src/include/storage/s_lock.h 22 Dec 2003 23:35:41 -0000 *************** *** 374,380 **** */ ! #if defined(__alpha) /* * Correct multi-processor locking methods are explained in section 5.5.3 --- 374,380 ---- */ ! #if defined(__alpha) || defined(__alpha__) /* * Correct multi-processor locking methods are explained in section 5.5.3 *************** *** 435,441 **** #endif /* defined(__GNUC__) */ ! #endif /* __alpha */ #if defined(__hppa) --- 435,441 ---- #endif /* defined(__GNUC__) */ ! #endif /* __alpha || __alpha__ */ #if defined(__hppa)
On Mon, Dec 22, 2003 at 06:36:32PM -0500, Bruce Momjian wrote: > Seems we have to test for __alpha and __alpha_. This applied patch > makes that consistent. Won't something like the following work? #ifdef(__alpha) #define __alpha__ 1 #endif so you only have to test for one of them? Just an idea. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) Si no sabes adonde vas, es muy probable que acabes en otra parte.
Alvaro Herrera wrote: > On Mon, Dec 22, 2003 at 06:36:32PM -0500, Bruce Momjian wrote: > > Seems we have to test for __alpha and __alpha_. This applied patch > > makes that consistent. > > Won't something like the following work? > > #ifdef(__alpha) > #define __alpha__ 1 > #endif > > so you only have to test for one of them? Just an idea. Yes, I thought of that. Where should we put it? c.h? -- 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
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> On Mon, Dec 22, 2003 at 06:36:32PM -0500, Bruce Momjian wrote:
>> Seems we have to test for __alpha and __alpha_. This applied patch
>> makes that consistent.
> Won't something like the following work?
> #ifdef(__alpha)
> #define __alpha__ 1
> #endif
It seems risky to me to define macros that are in the
reserved-for-system-use namespace. Who knows what might break in the
system headers if we did that?
I'm not convinced that all of the changes Bruce made are needed, or even
not likely to break things themselves. What if __alpha and __alpha__
actually indicate slightly different platforms or OS releases? For
example, we have *no* evidence to suggest that that NOFIXADE stuff in
main.c is needed on platforms that don't define __alpha. I would tend
to take an "if it ain't broke don't fix it" approach, especially on
platforms we don't have handy to test.
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > On Mon, Dec 22, 2003 at 06:36:32PM -0500, Bruce Momjian wrote: > >> Seems we have to test for __alpha and __alpha_. This applied patch > >> makes that consistent. > > > Won't something like the following work? > > > #ifdef(__alpha) > > #define __alpha__ 1 > > #endif > > It seems risky to me to define macros that are in the > reserved-for-system-use namespace. Who knows what might break in the > system headers if we did that? > > I'm not convinced that all of the changes Bruce made are needed, or even > not likely to break things themselves. What if __alpha and __alpha__ > actually indicate slightly different platforms or OS releases? For > example, we have *no* evidence to suggest that that NOFIXADE stuff in > main.c is needed on platforms that don't define __alpha. I would tend > to take an "if it ain't broke don't fix it" approach, especially on > platforms we don't have handy to test. The problem was that certain cases tested for __alpha__ and some __alpha --- same with __sparc. I think I might have gotten started with the these spinlock changes because of an __alpha fix. I also verified with an alpha guy that we need both in most cases, if not all. I remember trying to go with __alpha__ and finding someone couldn't compile alpha after that. -- 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 Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> example, we have *no* evidence to suggest that that NOFIXADE stuff in
>> main.c is needed on platforms that don't define __alpha. I would tend
>> to take an "if it ain't broke don't fix it" approach, especially on
>> platforms we don't have handy to test.
> The problem was that certain cases tested for __alpha__ and some
> __alpha --- same with __sparc.
My point is that without testing, you have no way to know that that
variation is not correct, and perhaps even necessary. Given that this
code has been through many releases already, I think the odds that you
are breaking something are higher than the odds that you are fixing
something. I think you should leave well enough alone until we get an
actual bug report showing that there's a problem.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> example, we have *no* evidence to suggest that that NOFIXADE stuff in > >> main.c is needed on platforms that don't define __alpha. I would tend > >> to take an "if it ain't broke don't fix it" approach, especially on > >> platforms we don't have handy to test. > > > The problem was that certain cases tested for __alpha__ and some > > __alpha --- same with __sparc. > > My point is that without testing, you have no way to know that that > variation is not correct, and perhaps even necessary. Given that this > code has been through many releases already, I think the odds that you > are breaking something are higher than the odds that you are fixing > something. I think you should leave well enough alone until we get an > actual bug report showing that there's a problem. OK, removed __alpha__ tests from main.c, and documented that they aren't being used, so we know it is a concious decision. I left the __alpha__ in s_lock.h because of course that is generic. -- 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 Momjian <pgman@candle.pha.pa.us> writes:
> OK, removed __alpha__ tests from main.c, and documented that they aren't
> being used, so we know it is a concious decision. I left the __alpha__
> in s_lock.h because of course that is generic.
Fair enough --- I was mostly worried about the NOFIXADE code, since
there's no one left around here who's got the foggiest idea what that
is for. My guess is that it is only needed on some ancient versions
of some Alpha platform or other ...
regards, tom lane
Tom Lane wrote:
> Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> > On Mon, Dec 22, 2003 at 06:36:32PM -0500, Bruce Momjian wrote:
> >> Seems we have to test for __alpha and __alpha_. This applied patch
> >> makes that consistent.
>
> > Won't something like the following work?
>
> > #ifdef(__alpha)
> > #define __alpha__ 1
> > #endif
>
> It seems risky to me to define macros that are in the
> reserved-for-system-use namespace. Who knows what might break in the
> system headers if we did that?
I see we already do this in solaris.h:
#if defined(__i386) && !defined(__i386__)
#define __i386__
#endif
#if defined(__sparc) && !defined(__sparc__)
#define __sparc__
#endif
--
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 Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> It seems risky to me to define macros that are in the
>> reserved-for-system-use namespace. Who knows what might break in the
>> system headers if we did that?
> I see we already do this in solaris.h:
Mph. Well, at least that's restricted to just one OS. It still seems
less than prudent to me, though.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> It seems risky to me to define macros that are in the > >> reserved-for-system-use namespace. Who knows what might break in the > >> system headers if we did that? > > > I see we already do this in solaris.h: > > Mph. Well, at least that's restricted to just one OS. It still seems > less than prudent to me, though. Agreed. -- 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