Re: Patch to support AMD64 Solaris 10

[Mark Kettenis <kettenis@gnu.org>]

   Date: Tue, 26 Oct 2004 19:43:59 +0000 (UTC)
   From: "Joseph S. Myers" <joseph@codesourcery.com>

   On Mon, 25 Oct 2004, Mark Kettenis wrote:

   > Thanks for your contribution.  The code looks pretty good to me, but
   > here are a few comments.

   This revised patch tries to take account of those comments.

Sorry Joseph, I didn't find the time yet to answer your previous mail
yet.  I wish I had, because some of the changes you made aren't
exactly going into the direction I'd want them to.

   >    The configuration is based on the existing IA32 Solaris support.
   > 
   > Fair enough, although it's not necessarily the best example.

   The configuration remains based on that one.

   > This sounds pretty much like the situation for Solaris SPARC.  I think
   > the way this is solved in sparc-sol2-tdep.c is pretty elegant, but I
   > may be biased ;-).  The defined(__arch64__) should be dropped though,
   > since there is no need to support Linux and IA-32/AMD64.  For
   > consistency with SPARC I'd suggest nameing the file i386-sol2-nat.c.

   It is now following sparc-sol2-nat.c - for that purpose I split 
   i386v4-nat.c into two files, one defining the functions under different 
   names so they can be used in this way and one defining the existing 
   functions as wrappers for those names.

The splitting of the file doesn't make me happy.  It'd be better if
the Solaris code didn't attempt to use i386v4-nat.c at all, but that
means some other changes are needed.  Big difference between SPARC and
AMD64 on one side and i386 on the other side is that the layout of the
prfpregset is defined in the -tdep.c file instead of the -nat.c file.
That's what I need to do for i386 too.  Please give me a few days to
set that right first.

Why did you use #ifdef __x86_64__ in i386-sol2-nat.c.  It's not
unlikely that compilers will get that wrong.  Please do this in a similar way to SPARC Solaris and use

#if defined (PR_MODEL_NATIVE) && (PR_MODEL_NATIVE == PR_MODEL_LP64)

(unless that doesn't work of course).

   > Comparison with Solaris SPARC makes me believe that using
   > gregset_t/fpregset_t in amd64-sol2-nat.c isn't right and that you
   > should use prgregset_t/prfpregset_t instead.

   I haven't made any changes in this area; i386v4-nat.c / i386v4-regset.c, 
   now used from this file, use gregset_t/fpregset_t themselves.

The Solaris-specific files should defenitely use
prgregset_t/prfpgregset_t.  It's the officially published API.

   > Again, for consistency with Solaris SPARC, could you name the makefile
   > fragments sol2-64.m[th] instead of sol64.m[th]?

   Now named i386sol2-65.m[th] given the existing naming of the IA32 Solaris 
   fragments.

The names of the IA32 Solaris fragments is historic.  We've been
moving to stripping the architecture from the name for quite some time
now; it's already encoded in the name of the directory.  So please use
the names I suggested.

Mark