Thread: gcc 4.0 build failures on m68k, mips, and mipsel
Hi! I noticed that PostgreSQL does not build any more with gcc 4.0 on m68k, mips, and mipsel because of bugs/incompatibilities in src/include/storage/s_lock.h. Fixing m68k was easy, I attach the patch. gcc 4.0 seems to define both __m68000___ and __m68k__, thus slock_t was defined twice. In 3.x, __m68k__ was not defined, as it seems. So the patch should work for both 3.x and 4.0. However, since I don't have access to a mips/mipsel machine with gcc 4.0, I was unable to fix the problem myself, so I just disabled spinlocks on these two architectures in Debian for now (which sucks, but at least the package builds and works). Does anybody here have a MIPS machine and could try this? I'd love to give links to the build logs, unfortunately some Debian servers are moving right now, thus buildd.debian.org is down. I'll followup with the logs as soon as it is up again. Thanks and have a nice day! Martin -- Martin Pitt http://www.piware.de Ubuntu Developer http://www.ubuntu.com Debian Developer http://www.debian.org
Attachment
Martin Pitt <martin@piware.de> writes:
> I noticed that PostgreSQL does not build any more with gcc 4.0 on
> m68k, mips, and mipsel because of bugs/incompatibilities in
> src/include/storage/s_lock.h.
> Fixing m68k was easy, I attach the patch.
This patch unquestionably breaks whatever platforms are using the
out-of-line m68k code in s_lock.c. Internal evidence there suggests
that at least netbsd will be affected.
Perhaps adding "if !defined(__linux__)" to the m68k tests would be better?
regards, tom lane
Hi again, Martin Pitt [2005-07-24 17:59 +0200]: > I noticed that PostgreSQL does not build any more with gcc 4.0 on > m68k, mips, and mipsel because of bugs/incompatibilities in > src/include/storage/s_lock.h. > [...] > However, since I don't have access to a mips/mipsel machine with gcc > 4.0, I was unable to fix the problem myself, so I just disabled > spinlocks on these two architectures in Debian for now (which sucks, > but at least the package builds and works). Does anybody here have a > MIPS machine and could try this? A better workaround than disabling spinlocks at all is to compile s_lock.c with -fno-unit-at-a-time (this is the default in gcc 4 and breaks it; that was the reason why it worked with gcc 3.3). However, Thiemo Seufer dealt with this and created a proper tas() function for PostgreSQL on mips, which also works with gcc 4. Of course this is much better, so I applied his patch in Debian. I attach the patch. Do you consider applying it upstream? Thanks and have a nice weekend, Martin -- Martin Pitt http://www.piware.de Ubuntu Developer http://www.ubuntu.com Debian Developer http://www.debian.org
Attachment
Martin Pitt <martin@piware.de> writes:
> However, Thiemo Seufer dealt with this and created a proper tas()
> function for PostgreSQL on mips, which also works with gcc 4. Of
> course this is much better, so I applied his patch in Debian.
> I attach the patch. Do you consider applying it upstream?
Hm, does this patch actually compile as-is? I'd think you'd need to
have the MIPS definition of slock_t placed earlier than the assembly
code fragment.
regards, tom lane
Hi! Tom Lane [2005-08-20 15:10 -0400]: > Martin Pitt <martin@piware.de> writes: > > However, Thiemo Seufer dealt with this and created a proper tas() > > function for PostgreSQL on mips, which also works with gcc 4. Of > > course this is much better, so I applied his patch in Debian. > > I attach the patch. Do you consider applying it upstream? > > Hm, does this patch actually compile as-is? I'd think you'd need to > have the MIPS definition of slock_t placed earlier than the assembly > code fragment. Sorry, I got the patch against 7.4 and I couldn't check it against 8.0 I fixed it for 8.0, and the new packages have finally built successfully on mips: http://buildd.debian.org/fetch.php?&pkg=postgresql-7.4&ver=1%3A7.4.8-17&arch=mips&stamp=1124942993&file=log&as=raw http://buildd.debian.org/fetch.php?&pkg=postgresql-8.0&ver=8.0.3-14&arch=mips&stamp=1124946041&file=log&as=raw I attach the updated patch against 8.0. Sorry for the confusion, and thanks! Martin -- Martin Pitt http://www.piware.de Ubuntu Developer http://www.ubuntu.com Debian Developer http://www.debian.org
Attachment
Martin Pitt <martin@piware.de> writes:
>>> However, Thiemo Seufer dealt with this and created a proper tas()
>>> function for PostgreSQL on mips, which also works with gcc 4. Of
>>> course this is much better, so I applied his patch in Debian.
>>> I attach the patch. Do you consider applying it upstream?
>>
>> Hm, does this patch actually compile as-is? I'd think you'd need to
>> have the MIPS definition of slock_t placed earlier than the assembly
>> code fragment.
> I attach the updated patch against 8.0.
OK, better, but I think not quite there yet. AFAICS the s_lock.c code
should simply go away, as there are no platforms for which it'd be used.
Also, the constraints for the inline assembler ought to include memory
and cc clobbers, according to the comments near the head of s_lock.h.
I've applied the attached modified patch to CVS HEAD, but can't test it
without a MIPS machine ... could you double-check it?
regards, tom lane
*** src/backend/storage/lmgr/s_lock.c.orig Fri Jul 29 23:07:40 2005
--- src/backend/storage/lmgr/s_lock.c Thu Aug 25 13:14:27 2005
***************
*** 172,205 ****
#endif /* __m68k__ */
- #if defined(__mips__) && !defined(__sgi)
- static void
- tas_dummy()
- {
- __asm__ __volatile__(
- "\
- .global tas \n\
- tas: \n\
- .frame $sp, 0, $31 \n\
- .set push \n\
- .set mips2 \n\
- ll $14, 0($4) \n\
- or $15, $14, 1 \n\
- sc $15, 0($4) \n\
- .set pop \n\
- beq $15, 0, fail\n\
- bne $14, 0, fail\n\
- li $2, 0 \n\
- .livereg 0x2000FF0E,0x00000FFF \n\
- j $31 \n\
- fail: \n\
- li $2, 1 \n\
- j $31 \n\
- ");
- }
- #endif /* __mips__ && !__sgi */
-
-
#else /* not __GNUC__ */
/*
--- 172,177 ----
*** src/include/storage/s_lock.h.orig Thu Mar 10 16:41:01 2005
--- src/include/storage/s_lock.h Thu Aug 25 13:14:21 2005
***************
*** 453,472 ****
#endif /* __alpha || __alpha__ */
! /* These live in s_lock.c, but only for gcc */
! #if defined(__m68k__)
! #define HAS_TEST_AND_SET
! typedef unsigned char slock_t;
! #endif
! #if defined(__mips__) && !defined(__sgi)
#define HAS_TEST_AND_SET
! typedef unsigned int slock_t;
#endif
--- 453,500 ----
#endif /* __alpha || __alpha__ */
! #if defined(__mips__) && !defined(__sgi)
! /* Note: on SGI we use the OS' mutex ABI, see below */
! #define HAS_TEST_AND_SET
+ typedef unsigned int slock_t;
! #define TAS(lock) tas(lock)
! static __inline__ int
! tas(volatile slock_t *lock)
! {
! register volatile slock_t *__l = lock;
! register int __r;
!
! __asm__ __volatile__(
! " .set push \n"
! " .set mips2 \n"
! " .set noreorder \n"
! " .set nomacro \n"
! "1: ll %0, %1 \n"
! " bne %0, $0, 1f \n"
! " xori %0, 1 \n"
! " sc %0, %1 \n"
! " beq %0, $0, 1b \n"
! " sync \n"
! "1: .set pop "
! : "=&r" (__r), "+R" (*__l)
! :
! : "memory", "cc");
! return __r;
! }
+ #endif /* __mips__ && !__sgi */
!
! /* These live in s_lock.c, but only for gcc */
!
!
! #if defined(__m68k__)
#define HAS_TEST_AND_SET
! typedef unsigned char slock_t;
#endif