This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc][1/2] add suport for 64-bit fpscr in powerpc linux native
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Thiago Jung Bauermann <bauerman at br dot ibm dot com>
- Cc: gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Thu, 13 Nov 2008 15:50:27 -0800
- Subject: Re: [rfc][1/2] add suport for 64-bit fpscr in powerpc linux native
- References: <1221436875.17278.4.camel@localhost.localdomain>
I will start by saying that I don't know the various powerpc variants
very well, but your patch sat unreviewed for a long time, so I have
to trust your judgement in the way you separated the ia205 variants
from the rest. From what I can tell, this is pretty much the standard
way of doing this, so this looks fine.
> gdb/
> * ppc-linux-nat.c (ppc_register_u_addr): Add special case to return
> offset for full 64-bit slot of FPSCR when in 32-bits.
> (ppc_linux_read_description): Return target description with 64-bit
> FPSCR when inferior is running on an ISA 2.05 or later processor.
> * ppc-linux-tdep.c (_initialize_ppc_linux_tdep): Call
> initialize_tdec_powerpc_isa205_32l,
> initialize_tdec_powerpc_isa205_altivec32l,
> initialize_tdec_powerpc_isa205_vsx32l,
> initialize_tdec_powerpc_isa205_64l,
> initialize_tdec_powerpc_isa205_altivec64l and
> initialize_tdec_powerpc_isa205_vsx64l.
> * ppc-linux-tdep.h: Add external declaration for
> tdesc_powerpc_isa205_32l, tdesc_powerpc_isa205_altivec32l,
> tdesc_powerpc_isa205_vsx32l, tdesc_powerpc_isa205_64l,
> tdesc_powerpc_isa205_altivec64l and tdesc_powerpc_isa205_vsx64l.
> * features/rs600/powerpc-fpu-isa205.xml: New file.
> * features/rs600/powerpc-isa205-32l.xml: New file.
> * features/rs600/powerpc-isa205-64l.xml: New file.
> * features/rs600/powerpc-isa205-altivec32l.xml: New file.
> * features/rs600/powerpc-isa205-altivec64l.xml: New file.
> * features/rs600/powerpc-isa205-vsx32l.xml: New file.
> * features/rs600/powerpc-isa205-vsx64l.xml: New file.
> * features/rs600/powerpc-isa205-32l.c: Generate.
> * features/rs600/powerpc-isa205-64l.c: Generate.
> * features/rs600/powerpc-isa205-altivec32l.c: Generate.
> * features/rs600/powerpc-isa205-altivec64l.c: Generate.
> * features/rs600/powerpc-isa205-vsx32l.c: Generate.
> * features/rs600/powerpc-isa205-vsx64l.c: Generate.
>
> gdb/testsuite/
>
> * gdb.arch/ppc-dfp.exp: New file.
> * gdb.arch/ppc-dfp.c: New file.
> +#ifndef PPC_FEATURE_ARCH_2_05
> +#define PPC_FEATURE_ARCH_2_05 0x00001000 /* ISA 2.05 */
> +#endif
How would this macro be defined? Does it come from one of the system
includes? What does this macro contain? Perhaps a comment describing
this macro would be useful.
(a few minutes later, after I started looking at the change inside
ppc_linux_read_description, I think I get it, now...)
> + /* If the FPSCR is 64-bit wide, we need to fetch the whole 64-bit
> + slot and not just its second word. The PT_FPSCR supplied in a
> + 32-bit GDB compilation doesn't reflect this. */
32-bit GDB compilation: Do you mean a 32-bit GDB?
While you're modifying this part of the code, I noticed a typo in
a comment just above:
/* NOTE: cagney/2005-02-08: On some 64-bit GNU/Linux systems the
kernel headers incorrectly contained the 32-bit definition of
PT_FPSCR. For the 32-bit definition, floating-point
registers occupy two 32-bit "slots", and the FPSCR lives in
the secondhalf of such a slot-pair (hence +1). For 64-bit,
^^^^^^^^^^ missing space between second and half.
Thanks!
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@prep.ai.mit.edu
Could you remove this part form your copyright header?
> +gdb_test "print/t \$fpscr" " = 10100000000000000000000000000000000" "FPSCR for round to nearest, ties toward zero rounding mode"
Just a casual remark, not a request for change, but I'm surprised
that you might find /t more useful in this case than /x ;-).
:REVIEWMAIL:
--
Joel