From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5942 invoked by alias); 17 Nov 2008 22:01:28 -0000 Received: (qmail 5479 invoked by uid 22791); 17 Nov 2008 22:01:26 -0000 X-Spam-Check-By: sourceware.org Received: from igw2.br.ibm.com (HELO igw2.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 17 Nov 2008 22:00:30 +0000 Received: from d24relay01.br.ibm.com (unknown [9.8.31.16]) by igw2.br.ibm.com (Postfix) with ESMTP id 7170717F5F8 for ; Mon, 17 Nov 2008 19:58:28 -0200 (BRDT) Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.18.232.46]) by d24relay01.br.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id mAHN03pt1613958 for ; Mon, 17 Nov 2008 20:00:03 -0300 Received: from d24av01.br.ibm.com (loopback [127.0.0.1]) by d24av01.br.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id mAHM0QtE028276 for ; Mon, 17 Nov 2008 20:00:27 -0200 Received: from [9.8.11.52] ([9.8.11.52]) by d24av01.br.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id mAHM0Opk028224; Mon, 17 Nov 2008 20:00:25 -0200 Subject: Re: [rfc][1/2] add suport for 64-bit fpscr in powerpc linux native From: Thiago Jung Bauermann To: Joel Brobecker Cc: gdb-patches ml In-Reply-To: <20081113235402.GA12802@adacore.com> References: <1221436875.17278.4.camel@localhost.localdomain> <20081113235027.GA12484@adacore.com> <20081113235402.GA12802@adacore.com> Content-Type: text/plain; charset=utf-8 Date: Tue, 18 Nov 2008 05:45:00 -0000 Message-Id: <1226959224.28256.38.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-11/txt/msg00449.txt.bz2 Thanks for your review, Joel! El jue, 13-11-2008 a las 15:54 -0800, Joel Brobecker escribió: > 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. Yes, that's my conclusion as well. I don't like the explosion in the number of target description .xml files this approach brings, but I thought about it for a while, and couldn't find a better way to do it. If linking to the expat library wasn't optional when building GDB, one option would be to provide just the power-fpu-isa205.xml file containing the feature description, and then generate the target description XML on the fly including the appropriate features, and have GDB parse that. > > +#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...) I added this comment, to make it clearer: +/* The PPC_FEATURE_* defines should be provided by . + If they aren't, we can provide them ourselves (their values are fixed + because they are part of the kernel ABI). They are used in the AT_HWCAP + entry of the AUXV. */ #ifndef PPC_FEATURE_BOOKE #define PPC_FEATURE_BOOKE 0x00008000 #endif +#ifndef PPC_FEATURE_ARCH_2_05 +#define PPC_FEATURE_ARCH_2_05 0x00001000 /* ISA 2.05 */ +#endif > > + /* 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? Right. I changed the sentence to read "The PT_FPSCR supplied when GDB is compiled as a 32-bit app doesn't reflect this.", I hope it's better. > 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. Sure, fixed that in the patch as well. > > +# 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? Oops, good catch. Removed. > > +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 ;-). I agree in general /x is more helpful, but in this case binary was easier because the FPSCR is just a bunch of flags, so I could directly see which were set and which weren't. Ok, I admit I still don't do my taxes in hex. :-) > Overall, the patch looks sane to me. Can you just wait a few more days, > say until Monday to give the real experts a chance to yell at me ;-), > and then commit? Great! If nobody yells, I'll commit the following tomorrow. -- []'s Thiago Jung Bauermann IBM Linux Technology Center 2008-11-17 Thiago Jung Bauermann 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. Index: gdb.git/gdb/features/rs6000/power-fpu-isa205.xml =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/features/rs6000/power-fpu-isa205.xml 2008-11-17 18:37:25.000000000 -0200 @@ -0,0 +1,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Index: gdb.git/gdb/features/rs6000/powerpc-isa205-32l.xml =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/features/rs6000/powerpc-isa205-32l.xml 2008-11-17 18:37:25.000000000 -0200 @@ -0,0 +1,17 @@ + + + + + + + + powerpc:common + + + + Index: gdb.git/gdb/features/rs6000/powerpc-isa205-64l.xml =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/features/rs6000/powerpc-isa205-64l.xml 2008-11-17 18:37:25.000000000 -0200 @@ -0,0 +1,17 @@ + + + + + + + + powerpc:common64 + + + + Index: gdb.git/gdb/features/rs6000/powerpc-isa205-altivec32l.xml =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/features/rs6000/powerpc-isa205-altivec32l.xml 2008-11-17 18:37:25.000000000 -0200 @@ -0,0 +1,19 @@ + + + + + + + + powerpc:common + + + + + Index: gdb.git/gdb/features/rs6000/powerpc-isa205-altivec64l.xml =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/features/rs6000/powerpc-isa205-altivec64l.xml 2008-11-17 18:37:25.000000000 -0200 @@ -0,0 +1,19 @@ + + + + + + + + powerpc:common64 + + + + + Index: gdb.git/gdb/features/rs6000/powerpc-isa205-vsx32l.xml =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/features/rs6000/powerpc-isa205-vsx32l.xml 2008-11-17 18:37:25.000000000 -0200 @@ -0,0 +1,20 @@ + + + + + + + + powerpc:common + + + + + + Index: gdb.git/gdb/features/rs6000/powerpc-isa205-vsx64l.xml =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/features/rs6000/powerpc-isa205-vsx64l.xml 2008-11-17 18:37:25.000000000 -0200 @@ -0,0 +1,20 @@ + + + + + + + + powerpc:common64 + + + + + + Index: gdb.git/gdb/ppc-linux-nat.c =================================================================== --- gdb.git.orig/gdb/ppc-linux-nat.c 2008-11-17 18:02:15.000000000 -0200 +++ gdb.git/gdb/ppc-linux-nat.c 2008-11-17 19:39:06.000000000 -0200 @@ -56,9 +56,16 @@ #define PT_TRAP 40 #endif +/* The PPC_FEATURE_* defines should be provided by . + If they aren't, we can provide them ourselves (their values are fixed + because they are part of the kernel ABI). They are used in the AT_HWCAP + entry of the AUXV. */ #ifndef PPC_FEATURE_BOOKE #define PPC_FEATURE_BOOKE 0x00008000 #endif +#ifndef PPC_FEATURE_ARCH_2_05 +#define PPC_FEATURE_ARCH_2_05 0x00001000 /* ISA 2.05 */ +#endif /* Glibc's headers don't define PTRACE_GETVRREGS so we cannot use a configure time check. Some older glibc's (for instance 2.2.1) @@ -274,11 +281,17 @@ ppc_register_u_addr (struct gdbarch *gdb 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, + the second half of such a slot-pair (hence +1). For 64-bit, the FPSCR instead occupies the full 64-bit 2-word-slot and hence no adjustment is necessary. Hack around this. */ if (wordsize == 8 && PT_FPSCR == (48 + 32 + 1)) u_addr = (48 + 32) * wordsize; + /* 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 when + GDB is compiled as a 32-bit app doesn't reflect this. */ + else if (wordsize == 4 && register_size (gdbarch, regno) == 8 + && PT_FPSCR == (48 + 2*32 + 1)) + u_addr = (48 + 2*32) * wordsize; else u_addr = PT_FPSCR * wordsize; } @@ -1230,6 +1243,7 @@ ppc_linux_read_description (struct targe { int altivec = 0; int vsx = 0; + int isa205 = 0; int tid = TIDGET (inferior_ptid); if (tid == 0) @@ -1274,6 +1288,9 @@ ppc_linux_read_description (struct targe perror_with_name (_("Unable to fetch AltiVec registers")); } + if (ppc_linux_get_hwcap () & PPC_FEATURE_ARCH_2_05) + isa205 = 1; + /* Check for 64-bit inferior process. This is the case when the host is 64-bit, and in addition the top bit of the MSR register is set. */ #ifdef __powerpc64__ @@ -1284,21 +1301,21 @@ ppc_linux_read_description (struct targe if (errno == 0 && msr < 0) { if (vsx) - return tdesc_powerpc_vsx64l; + return isa205? tdesc_powerpc_isa205_vsx64l : tdesc_powerpc_vsx64l; else if (altivec) - return tdesc_powerpc_altivec64l; + return isa205? tdesc_powerpc_isa205_altivec64l : tdesc_powerpc_altivec64l; - return tdesc_powerpc_64l; + return isa205? tdesc_powerpc_isa205_64l : tdesc_powerpc_64l; } } #endif if (vsx) - return tdesc_powerpc_vsx32l; + return isa205? tdesc_powerpc_isa205_vsx32l : tdesc_powerpc_vsx32l; else if (altivec) - return tdesc_powerpc_altivec32l; + return isa205? tdesc_powerpc_isa205_altivec32l : tdesc_powerpc_altivec32l; - return tdesc_powerpc_32l; + return isa205? tdesc_powerpc_isa205_32l : tdesc_powerpc_32l; } void _initialize_ppc_linux_nat (void); Index: gdb.git/gdb/ppc-linux-tdep.c =================================================================== --- gdb.git.orig/gdb/ppc-linux-tdep.c 2008-11-17 18:02:15.000000000 -0200 +++ gdb.git/gdb/ppc-linux-tdep.c 2008-11-17 18:37:25.000000000 -0200 @@ -42,9 +42,15 @@ #include "features/rs6000/powerpc-32l.c" #include "features/rs6000/powerpc-altivec32l.c" #include "features/rs6000/powerpc-vsx32l.c" +#include "features/rs6000/powerpc-isa205-32l.c" +#include "features/rs6000/powerpc-isa205-altivec32l.c" +#include "features/rs6000/powerpc-isa205-vsx32l.c" #include "features/rs6000/powerpc-64l.c" #include "features/rs6000/powerpc-altivec64l.c" #include "features/rs6000/powerpc-vsx64l.c" +#include "features/rs6000/powerpc-isa205-64l.c" +#include "features/rs6000/powerpc-isa205-altivec64l.c" +#include "features/rs6000/powerpc-isa205-vsx64l.c" #include "features/rs6000/powerpc-e500l.c" @@ -1170,8 +1176,14 @@ _initialize_ppc_linux_tdep (void) initialize_tdesc_powerpc_32l (); initialize_tdesc_powerpc_altivec32l (); initialize_tdesc_powerpc_vsx32l (); + initialize_tdesc_powerpc_isa205_32l (); + initialize_tdesc_powerpc_isa205_altivec32l (); + initialize_tdesc_powerpc_isa205_vsx32l (); initialize_tdesc_powerpc_64l (); initialize_tdesc_powerpc_altivec64l (); initialize_tdesc_powerpc_vsx64l (); + initialize_tdesc_powerpc_isa205_64l (); + initialize_tdesc_powerpc_isa205_altivec64l (); + initialize_tdesc_powerpc_isa205_vsx64l (); initialize_tdesc_powerpc_e500l (); } Index: gdb.git/gdb/ppc-linux-tdep.h =================================================================== --- gdb.git.orig/gdb/ppc-linux-tdep.h 2008-11-17 18:02:15.000000000 -0200 +++ gdb.git/gdb/ppc-linux-tdep.h 2008-11-17 18:37:25.000000000 -0200 @@ -42,9 +42,15 @@ int ppc_linux_trap_reg_p (struct gdbarch extern struct target_desc *tdesc_powerpc_32l; extern struct target_desc *tdesc_powerpc_altivec32l; extern struct target_desc *tdesc_powerpc_vsx32l; +extern struct target_desc *tdesc_powerpc_isa205_32l; +extern struct target_desc *tdesc_powerpc_isa205_altivec32l; +extern struct target_desc *tdesc_powerpc_isa205_vsx32l; extern struct target_desc *tdesc_powerpc_e500l; extern struct target_desc *tdesc_powerpc_64l; extern struct target_desc *tdesc_powerpc_altivec64l; extern struct target_desc *tdesc_powerpc_vsx64l; +extern struct target_desc *tdesc_powerpc_isa205_64l; +extern struct target_desc *tdesc_powerpc_isa205_altivec64l; +extern struct target_desc *tdesc_powerpc_isa205_vsx64l; #endif /* PPC_LINUX_TDEP_H */ Index: gdb.git/gdb/testsuite/gdb.arch/ppc-dfp.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/testsuite/gdb.arch/ppc-dfp.c 2008-11-17 18:37:25.000000000 -0200 @@ -0,0 +1,46 @@ +/* Copyright 2008 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +#ifdef __powerpc64__ +typedef Elf64_auxv_t auxv_t; +#else +typedef Elf32_auxv_t auxv_t; +#endif + +#ifndef PPC_FEATURE_HAS_DFP +#define PPC_FEATURE_HAS_DFP 0x00000400 +#endif + +int +main (int argc, char *argv[], char *envp[], auxv_t auxv[]) +{ + int i; + + for (i = 0; auxv[i].a_type != AT_NULL; i++) + if (auxv[i].a_type == AT_HWCAP) { + if (!(auxv[i].a_un.a_val & PPC_FEATURE_HAS_DFP)) + return 1; + + break; + } + + asm ("mtfsfi 7, 5, 1\n"); /* Set DFP rounding mode. */ + + return 0; +} Index: gdb.git/gdb/testsuite/gdb.arch/ppc-dfp.exp =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb.git/gdb/testsuite/gdb.arch/ppc-dfp.exp 2008-11-17 19:41:51.000000000 -0200 @@ -0,0 +1,79 @@ +# Copyright (C) 2008 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# Tests for Powerpc Decimal Floating Point registers setting and fetching + +if $tracelevel then { + strace $tracelevel +} + +if ![istarget "powerpc*"] then { + verbose "Skipping powerpc decimal floating point register tests." + return +} + +set testfile "ppc-dfp" +set binfile ${objdir}/${subdir}/${testfile} +set srcfile ${testfile}.c + +if [get_compiler_info $binfile] { + warning "get_compiler failed" + return -1 +} + +if ![test_compiler_info gcc*] { + # We use GCC's extended asm syntax + warning "unknown compiler" + return -1 +} + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {quiet debug}] != "" } { + unsupported "This machine doesn't support Decimal Floating Point." + return -1 +} + +# Start with a fresh gdb. + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load ${binfile} + +gdb_breakpoint [gdb_get_line_number "Set DFP rounding mode."] + +gdb_run_cmd + +# When the prompt comes back we'll be at the Set DFP rounding mode breakpoint. +# Unless the program bails out after checking AT_HWCAP. +gdb_expect { + -re "Program exited with code 01.\[\r\n\]+$gdb_prompt $" { + unsupported "This machine doesn't support Decimal Floating Point." + return -1 + } + + -re ".*$gdb_prompt $" {} +} + +# First, verify if FPSCR is all zeroes. +gdb_test "print \$fpscr" " = 0" "FPSCR is all zeroes" + +# Step over "set rounding mode" instruction. +gdb_test "next" "" "" + +# Verify that the following bits are set (See Power ISA for details): +# +# 29:31 - DFP Rounding Control +gdb_test "print/t \$fpscr" " = 10100000000000000000000000000000000" "FPSCR for round to nearest, ties toward zero rounding mode"