The summary is self-explicit. I will attach patches to make this build work with the Sun studio 9 beta compiler which supports amd64. Later next week I will also add patches to build with gcc3.4.1 .
Severity: normal → enhancement
Priority: -- → P2
Target Milestone: --- → 4.5.1
Version: 4.5.1 → 4.5
This new file is nearly identical to the Linux AMD64 assembly file, except for : a) symbol name changes to match _solaris.h b) GNU directives removed
Forgot to mention that the patch uses the USE_64 environment variable to select 64-bit mode, just like on Sparc.
Julien, what's the output of "uname -a", "uname -m", and "uname -p" on Solaris AMD64 and Solaris x86? I'm wondering if we should use "uname -p" instead of "uname -m" to determine the processor type on Solaris. I guess "uname -p" should be able to distinguish between AMD64 and x86. Then, we would not need to use the USE_64 make variable.
Wan-Teh, here is the uname output . The -a is different, but -p and -m are the same . I have been told to use the command "isainfo" to automatically differentiate, which we don't currently use in our build system on any platform, so I didn't want to use that for the detection. I think it's useful to be able to choose whether to compile 32 bit or 64 bit, since the compilers (Sun cc and gcc) have the ability to do both. So, I think USE_64 is useful to preserve. For example, I couldn't figure out how to compile 32 bit on a Linux 64-bit box, which I think is very useful for comparison testing. I was forced to use a 32-bit Linux box to do the compilation. I could only target 64-bit on 64 Linux with our current build system. The following is the output for a 64-bit box : % uname -a SunOS nss-amd-solaris 5.10 amd64-gate-2004-09-08 i86pc i386 i86pc % uname -p i386 % uname -m i86pc % isainfo amd64 i386 On a 32-bit machine : % uname -a SunOS nss-sol-x86 5.9 Generic_112234-07 i86pc i386 i86pc % uname -p i386 % uname -m i86pc % isainfo i386 %
1) correct ifdef in solaris.h 2) fix alignment and endianness in solaris64.cfg
Attachment #158659 - Attachment is obsolete: true
Note that NSPR also builds with 64-bit gcc 3.4.1 on Solaris if NS_USE_GCC is set to 1. No changes were necessary to accomplish that.
actually the -P option isn't supported by the latest as from gcc 3.4.1 . Had to conditionally remove it for Solaris x86.
Attachment #159138 - Attachment is obsolete: true
Comment on attachment 159515 [details] [diff] [review] fix problem with gas Hi Julien, You need to modify mozilla/nsprpub/configure.in and regenerate mozilla/nsprpub/configure. For now, I'll review your direct changes to mozilla/nsprpub/configure. 1. mozilla/nsprpub/configure Please adjust the indentation when you add "if" around existing code. >+ if test "$OS_TEST" = "i86pc"; then >+ ASFLAGS="$ASFLAGS -Wa" >+ else > ASFLAGS="$ASFLAGS -Wa,-P" >+ fi Do you know what the -P option does? Why do we need it for x86-64? >+ if test "$OS_TEST" = "i86pc"; then >+ CC="$CC -xarch=amd64" >+ CXX="$CXX -xarch=amd64" >+ DEFINES="$DEFINES -D__x86_64__" >+ else Does Sun's compiler not define __x86_64__ implicitly? > if test "$OS_TEST" = "i86pc"; then >+ if test -n "$USE_64"; then >+ PR_MD_ASFILES=os_SunOS_x86_64.s >+ OS_TEST = x86_64 >+ else > PR_MD_ASFILES=os_SunOS_x86.s >+ fi Do we really need to change OS_TEST to x86_64 like this? This kind of change is very confusing. I don't think this change is necessary. 2. mozilla/nsprpub/pr/include/md/_solaris.h > #if defined(_PR_GLOBAL_THREADS_ONLY) || defined(_PR_PTHREADS) > /* > * We have assembly language implementation of atomic > * stacks for the 32-bit sparc and x86 architectures only. > * > * Note: We ran into thread starvation problem with the > * 32-bit sparc assembly language implementation of atomic > * stacks, so we do not use it now. (Bugzilla bug 113740) > */ >-#if !defined(sparc) >+#if !defined(sparc) && !defined(__x86_64__) > #define _PR_HAVE_ATOMIC_CAS > #endif > #endif As we discussed before, we probably should not use the x86 assembly implementation of atomic stacks either. If you don't want to do that in this patch, that's fine by me. 3. mozilla/nsprpub/pr/include/md/_solaris64.cfg >@@ -46,20 +46,25 @@ > #if defined(sparc) || defined(__sparc) > #undef IS_LITTLE_ENDIAN > #define IS_BIG_ENDIAN 1 > #define PR_ALIGN_OF_INT64 8 > #define PR_ALIGN_OF_DOUBLE 8 > #elif defined(i386) || defined(__i386) > #define IS_LITTLE_ENDIAN 1 > #undef IS_BIG_ENDIAN > #define PR_ALIGN_OF_INT64 4 > #define PR_ALIGN_OF_DOUBLE 4 >+#elif defined(__x86_64__) >+#define IS_LITTLE_ENDIAN 1 >+#undef IS_BIG_ENDIAN >+#define PR_ALIGN_OF_INT64 8 >+#define PR_ALIGN_OF_DOUBLE 8 > #else > #error unknown processor > #endif I encourage you to remove the i386/__i386 section from this file because it is not used. (This file is used by 64-bit architectures only.)
Attachment #159515 - Flags: superreview?(wchang0222) → superreview-
1. - OK, I'll look at the configure.in changes - I don't know what the -P option does. The problem actually is that it's not supported on x86-64 with gcc 3.4.1, and makes the as command fail. The as version is the following : [jp96085@nss-amd-solaris]/home/jp96085 83 % /opt/gcc-3.4.1/bin/as -V GNU assembler version 2.15 (i386-pc-solaris2.10) using BFD version 2.15 - No, Sun's studio 9 compiler doesn't define __x86_64__ implicitly . There may be another symbol to check for, but I haven't found it yet. There is serious network trouble today and I can't get to the compiler folks' web page. - I don't think the OS_TEST change is necessary, that was just a leftover from an early port attempt. 2. This might be better dealt with in a separate bug. 3. OK, I will remove the 386 section.
I found that the -P option is to invoke the C preprocessor on the assembly code. That appears to only be supported by Sun's as, not by GNU as . When assembling with gcc, GNU as is automatically invoked, even if Sun's as is in the PATH. The solution is to not pass that -P option when using gcc (ifdef NS_USE_GCC) .
- I updated configure.in, but couldn't test it (no autoconf on my AMD64 Solaris box). The changes should be the same as for the attached configure, though. - for the -P issue, I changed the tests to set GNU_AS to true if GCC is used as the compiler . The -Wa,-P arguments only get set when using the Sun assembler, so they don't conflict. I had to move the test for it a couple lines down.
Attachment #159515 - Attachment is obsolete: true
Comment on attachment 159543 [details] [diff] [review] patch for NSPR_4_5_BRANCH This patch is not obsolete, sorry for the confusion.
Comment on attachment 159543 [details] [diff] [review] patch for NSPR_4_5_BRANCH 1. In mozilla/nsprpub/configure.in > if test -n "$GNU_CC"; then > CFLAGS="$CFLAGS -Wall" > CXXFLAGS="$CXXFLAGS -Wall" >+ GNU_AS=1 (Please do not use the tab character.) Instead of hardcoding GNU_AS to 1, please use the following test: GCC_AS=`$CC -print-prog-name=as` if test "`echo | $GCC_AS -v 2>&1 | grep -c GNU`" != "0"; then GNU_AS=1 fi > if test "$OS_TEST" = "i86pc"; then > AC_DEFINE(i386) > CPU_ARCH_TAG=_$OS_TEST > # The default debug format, DWARF (-g), is not supported by gcc > # on i386-ANY-sysv4/solaris, but the stabs format is. It is I think the above test needs to be modified to also test for $USE_64. 2. In pr/include/md/_solaris.h, pr/include/md/_solaris64.cfg, and pr/src/io/prprf.c, we should test for __x86_64 instead of __x86_64__ because __x86_64 is implicitly defined by both compilers.
Attachment #159543 - Flags: superreview?(wchang0222) → superreview-
Comment on attachment 159678 [details] [diff] [review] patch for NSPR tip When you compile with "cc -xarch=amd64" on AMD64, does it implicitly define the i386 macro (or __i386, __i386__)?
Wan-Teh, Thanks for the review. 1. I didn't use this test, because for some unknown reason, running as -v currently hangs with the tools I have. This is probably because of the beta tools I'm currently using. I had tried this approach before I submitted the last patch. I'll check if the 386 section really needs to be modified. I don't think that's the case, because the bits run 2. I had chosen to use __x86_64__ for the tests because of the precedent of doing so on Linux with gcc, but if __x86_64 is also defined by gcc, then it makes sense to switch to this new test. I will submit a new patch.
In response to your other questions about the macros defined : For the Sun studio9 compiler, in 32-bit mode (no xarch specified), i386 is defined, and __i386 or __i386__ are not defined. With -xarch=amd64, none of these three macros are defined. With gcc, all 3 macros are defined, whether compiling 32-bit or 64 (-m64) ... :-(
I found out that calling as -v or as -V invokes the assembler by telling it not only to print the version, but also to process source code from stdin. --version is the correct option to use for this test. It works with both Solaris as and GNU as .
Wan-Teh, So far, I have been making manual changes to both configure and configure.in, but this is very tedious. I got a copy of autoconf 2.12 for Solaris, and tried to regenerate configure from configure.in, but it seems there are major differences with the previous configure. What version of autoconf is the "official one" to use ?
1) use version test for AS 2) check for USE_64 around AC_DEFINE(i386) 3) use __x86_64 instead of __x86_64__ macro
Attachment #159678 - Attachment is obsolete: true
Comment on attachment 161116 [details] [diff] [review] updated patch for tip r=wtc. In nsprpub/configure.in > if test "$OS_TEST" = "i86pc"; then >- AC_DEFINE(i386) >+ if test -z "$USE_64"; then >+ AC_DEFINE(i386) >+ fi > CPU_ARCH_TAG=_$OS_TEST > # The default debug format, DWARF (-g), is not supported by gcc > # on i386-ANY-sysv4/solaris, but the stabs format is. It is I'm wondering if we should also put _DEBUG_FLAGS=-gstabs (which is right below the code snippet shown above) inside if test -z "$USE_64".
Attachment #161116 - Flags: superreview?(wchang0222) → superreview+
Julien, I just noticed that you also changed the GNU_AS test for the Solaris/gcc case. As you observed in comment 19, $GCC_AS -v processes source code from stdin. This is why there is an "echo |" before it. You should either use the original test, or remove the "echo |".
The 'as' on my Solaris 8 system does not understand the --version flag. Its version is "as: Sun WorkShop 6 99/08/18". So I suggest going back to the GNU_AS test I proposed in comment 15. I am using autoconf 2.13 to generate configure.
Wan-Teh, I tried your suggestion yesterday for the stabs, but it didn't appear to make a difference either way. The build succeeded with or without this test, in either 32 or 64 bit mode. I did not attempt to debug the assembly code, though. Regarding the assembler test, I hadn't noticed the echo . Please change the --version to -V (capital V) for the checkin to the tip, unless you want me to generate a new patch just for this issue. I will make a new patch for the 3.9 branch shortly.
Uses -V instead of --version in the test
Assignee: wchang0222 → julien.pierre.bugs
Attachment #161116 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Same changes as for the tip patch
Comment on attachment 161205 [details] [diff] [review] Update for NSPR_4_5_BRANCH Wan-Teh, Please review. I'd like to check this in to NSPR_4_5_BRANCH today. Thanks.
Comment on attachment 161204 [details] [diff] [review] Update for tip Please check this in to the NSPR tip. Thanks !
Attachment #161204 - Flags: review?(wchang0222)
Comment on attachment 161204 [details] [diff] [review] Update for tip I checked in this patch on the NSPR tip after some minor changes. 1. I still use -v (lowercase v) to be consistent with the other GNU_AS test in configure.in. Either -v or -V will work for the GNU_AS test because the GNU as supports both. (Solaris as only supports -V, but we are not trying to detect Solaris as here.) 2. I made some white space changes. 3. In _solaris.h, I check __x86_64 before I check i386. This is because you said that some compilers implicitly define i386 on AMD64 64-bit build. If this is true, we need to review all checks for i386 in our source code. Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 1.170; previous revision: 1.169 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 1.175; previous revision: 1.174 done Checking in pr/include/md/_solaris.h; /cvsroot/mozilla/nsprpub/pr/include/md/_solaris.h,v <-- _solaris.h new revision: 3.27; previous revision: 3.26 done Checking in pr/include/md/_solaris64.cfg; /cvsroot/mozilla/nsprpub/pr/include/md/_solaris64.cfg,v <-- _solaris64.cfg new revision: 3.7; previous revision: 3.6 done RCS file: /cvsroot/mozilla/nsprpub/pr/src/md/unix/os_SunOS_x86_64.s,v done Checking in pr/src/md/unix/os_SunOS_x86_64.s; /cvsroot/mozilla/nsprpub/pr/src/md/unix/os_SunOS_x86_64.s,v <-- os_SunOS_x86_6 4.s initial revision: 1.1 done
Attachment #161204 - Flags: review?(wchang0222) → review+
Thanks, Wan-Teh. I checked in the patch with your changes 1 and 3 to the NSPR_4_5_BRANCH . Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 220.127.116.11; previous revision: 18.104.22.168 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 22.214.171.124; previous revision: 126.96.36.199 done Checking in pr/include/md/_solaris.h; /cvsroot/mozilla/nsprpub/pr/include/md/_solaris.h,v <-- _solaris.h new revision: 188.8.131.52; previous revision: 3.25 done Checking in pr/include/md/_solaris64.cfg; /cvsroot/mozilla/nsprpub/pr/include/md/_solaris64.cfg,v <-- _solaris64.cfg new revision: 184.108.40.206; previous revision: 3.5 done Checking in pr/src/io/prprf.c; /cvsroot/mozilla/nsprpub/pr/src/io/prprf.c,v <-- prprf.c new revision: 220.127.116.11; previous revision: 3.14 done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You missed the new file os_SunOS_x86_64.s. I added that to NSPR_4_5_BRANCH for you. I also regenerated configure using the official autoconf 2.13 (not the autoconf-2.13 on Red Hat Enterprise Linux 3) with a manual change to eliminate \015. (This is needed for configure to work under MKS Korn shell.)
Thanks for adding that file ! FYI, the autoconf bits that came pre-installed on our RHAS3 box here (with the AMD64 linux kernel) is version 2.57 . I would appreciate a pointer to the 2.13 bits for future use.
You need to log in before you can comment on or make changes to this bug.