259001 - port NSPR to Solaris on AMD64 architecture

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement, P2)

Description

[Julien Pierre]

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 .

Comment 1

[Julien Pierre]

Attachment: initial patch to compile on Solaris AMD64

Comment 2

[Julien Pierre]

Attachment: assembly file for Solaris AMD64

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

Comment 3

[Julien Pierre]

Forgot to mention that the patch uses the USE_64 environment variable to select
64-bit mode, just like on Sparc.

Comment 4

[Wan-Teh Chang]

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.

Comment 5

[Julien Pierre]

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
%

Comment 6

[Julien Pierre]

Attachment: updated patch

1) correct ifdef in solaris.h
2) fix alignment and endianness in solaris64.cfg

Comment 7

[Julien Pierre]

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.

Comment 8

[Julien Pierre]

Attachment: fix problem with gas

actually the -P option isn't supported by the latest as from gcc 3.4.1 . Had to
conditionally remove it for Solaris x86.

Comment 9

[Wan-Teh Chang]

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.)

Comment 10

[Julien Pierre]

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.

Comment 11

[Julien Pierre]

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) .

Comment 12

[Julien Pierre]

Attachment: patch for NSPR_4_5_BRANCH

- 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.

Comment 13

[Julien Pierre]

Attachment: patch for NSPR tip

Comment 14

[Julien Pierre]

Comment on attachment 159543 [details] [diff] [review]
patch for NSPR_4_5_BRANCH

This patch is not obsolete, sorry for the confusion.

Comment 15

[Wan-Teh Chang]

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.

Comment 16

[Wan-Teh Chang]

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__)?

Comment 17

[Julien Pierre]

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.

Comment 18

[Julien Pierre]

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) ... :-(

Comment 19

[Julien Pierre]

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 .

Comment 20

[Julien Pierre]

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 ?

Comment 21

[Julien Pierre]

Attachment: updated patch for tip

1) use version test for AS
2) check for USE_64 around AC_DEFINE(i386)
3) use __x86_64 instead of __x86_64__ macro

Comment 22

[Wan-Teh Chang]

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

Comment 23

[Wan-Teh Chang]

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

Comment 24

[Wan-Teh Chang]

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.

Comment 25

[Julien Pierre]

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.

Comment 26

[Julien Pierre]

Attachment: Update for tip

Uses -V instead of --version in the test

Comment 27

[Julien Pierre]

Attachment: Update for NSPR_4_5_BRANCH

Same changes as for the tip patch

Comment 28

[Julien Pierre]

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 29

[Julien Pierre]

Comment on attachment 161204 [details] [diff] [review]
Update for tip

Please check this in to the NSPR tip. Thanks !

Comment 30

[Wan-Teh Chang]

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

Comment 31

[Julien Pierre]

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: 1.161.2.3; previous revision: 1.161.2.2
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.165.2.3; previous revision: 1.165.2.2
done
Checking in pr/include/md/_solaris.h;
/cvsroot/mozilla/nsprpub/pr/include/md/_solaris.h,v  <--  _solaris.h
new revision: 3.25.2.1; 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: 3.5.8.1; 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: 3.14.2.1; previous revision: 3.14
done

Comment 32

[Wan-Teh Chang]

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.)

Comment 33

[Julien Pierre]

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.