From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24274 invoked by alias); 25 Mar 2009 21:22:55 -0000 Received: (qmail 24262 invoked by uid 22791); 25 Mar 2009 21:22:54 -0000 X-SWARE-Spam-Status: No, hits=-2.8 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from e24smtp01.br.ibm.com (HELO e24smtp01.br.ibm.com) (32.104.18.85) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 25 Mar 2009 21:22:48 +0000 Received: from d24relay01.br.ibm.com (d24relay01.br.ibm.com [9.8.31.16]) by e24smtp01.br.ibm.com (8.13.1/8.13.1) with ESMTP id n2PLYeQ1028651 for ; Wed, 25 Mar 2009 18:34:40 -0300 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.2) with ESMTP id n2PMM3uF3776566 for ; Wed, 25 Mar 2009 19:22: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 n2PLMg5x025460 for ; Wed, 25 Mar 2009 18:22:42 -0300 Received: from [9.8.3.21] ([9.8.3.21]) by d24av01.br.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id n2PLMftX025454; Wed, 25 Mar 2009 18:22:41 -0300 Subject: Re: [RFA] Change AUXV bit checked to decide the size of the FPSCR From: Thiago Jung Bauermann To: Joel Brobecker Cc: gdb-patches ml In-Reply-To: <20090325163938.GY9472@adacore.com> References: <1237819493.25721.38.camel@localhost.localdomain> <20090325163938.GY9472@adacore.com> Content-Type: text/plain; charset=utf-8 Date: Wed, 25 Mar 2009 21:27:00 -0000 Message-Id: <1238016161.25721.76.camel@localhost.localdomain> Mime-Version: 1.0 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: 2009-03/txt/msg00564.txt.bz2 El mié, 25-03-2009 a las 09:39 -0700, Joel Brobecker escribió: > (Thiago - I like the quality of the work you produce, very nice!) Many thanks! Also thanks for the quick review. After I push enough Python patches upstream, I hope I can try to do some patch review work too, we will see then if I am any good at it. :-) > > For this reason, I'm changing GDB to check for DFP to decide what is the > > size of the FPSCR (it changed from 32 bits to 64 bits with ISA 2.05 and > > newer). Since for now the only higher bits used are for Decimal Floating > > Point, I am changing the code to check the DFP bit in AUXV. > > This sounds more like a work-around than a real fix. I agree. Truth be told, IMHO the right fix would be to have this bit set in AUXV for the Power 7. But changing it at this point would have consequences, and besides I don't think I'm in a position to fight this. > I don't mind > your approach, but I think it deserves a clear comment in the code > where you use that flag to determine the size of your registers. Great. I added this comment: /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only PPC_FEATURE_ARCH_2_06. Since for now the only bits used in the higher half of the register are for Decimal Floating Point, we check if that feature is available to decide the size of the FPSCR. */ > For the record, another solution that you probably considered was to > check for PPC_FEATURE_ARCH_2_05 *or* PPC_FEATURE_ARCH_2_06, but I can > see how this might become cumbersome as future revisions get added. Yes, it would get ugly with time. I don't think DFP support will be dropped anytime soon, so checking for the feature will work for a good while. > > gdb/ > > * ppc-linux-nat.c (PPC_FEATURE_ARCH_2_05): Remove #define. > > (PPC_FEATURE_HAS_DFP): New #define. > > (ppc_linux_read_description): Check for DFP feature instead of > > ISA 2.05 to decide on size of the FPSCR. > > > > gdbserver/ > > * linux-ppc-low.c (PPC_FEATURE_ARCH_2_05): Remove #define. > > (PPC_FEATURE_HAS_DFP): New #define. > > (ppc_arch_setup): Check for DFP feature instead of ISA 2.05 to decide on > > size of the FPSCR. > > Both OK with the requested comments added. Committed the following. -- []'s Thiago Jung Bauermann IBM Linux Technology Center 2009-03-25 Thiago Jung Bauermann gdb/ * ppc-linux-nat.c (PPC_FEATURE_ARCH_2_05): Remove #define. (PPC_FEATURE_HAS_DFP): New #define. (ppc_linux_read_description): Check for DFP feature instead of ISA 2.05 to decide on size of the FPSCR. gdbserver/ * linux-ppc-low.c (PPC_FEATURE_ARCH_2_05): Remove #define. (PPC_FEATURE_HAS_DFP): New #define. (ppc_arch_setup): Check for DFP feature instead of ISA 2.05 to decide on size of the FPSCR. Index: src/gdb/gdbserver/linux-ppc-low.c =================================================================== --- src.orig/gdb/gdbserver/linux-ppc-low.c 2009-02-02 22:12:16.000000000 -0200 +++ src/gdb/gdbserver/linux-ppc-low.c 2009-03-25 18:10:05.000000000 -0300 @@ -28,7 +28,7 @@ #define PPC_FEATURE_HAS_VSX 0x00000080 #define PPC_FEATURE_HAS_ALTIVEC 0x10000000 #define PPC_FEATURE_HAS_SPE 0x00800000 -#define PPC_FEATURE_ARCH_2_05 0x00001000 +#define PPC_FEATURE_HAS_DFP 0x00000400 static unsigned long ppc_hwcap; @@ -274,14 +274,21 @@ ppc_arch_setup (void) ppc_get_hwcap (&ppc_hwcap); if (ppc_hwcap & PPC_FEATURE_HAS_VSX) { - if (ppc_hwcap & PPC_FEATURE_ARCH_2_05) + /* Power ISA 2.05 (implemented by Power 6 and newer processors) + increases the FPSCR from 32 bits to 64 bits. Even though Power 7 + supports this ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 + set, only PPC_FEATURE_ARCH_2_06. Since for now the only bits + used in the higher half of the register are for Decimal Floating + Point, we check if that feature is available to decide the size + of the FPSCR. */ + if (ppc_hwcap & PPC_FEATURE_HAS_DFP) init_registers_powerpc_isa205_vsx64l (); else init_registers_powerpc_vsx64l (); } else if (ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC) { - if (ppc_hwcap & PPC_FEATURE_ARCH_2_05) + if (ppc_hwcap & PPC_FEATURE_HAS_DFP) init_registers_powerpc_isa205_altivec64l (); else init_registers_powerpc_altivec64l (); @@ -297,14 +304,14 @@ ppc_arch_setup (void) ppc_get_hwcap (&ppc_hwcap); if (ppc_hwcap & PPC_FEATURE_HAS_VSX) { - if (ppc_hwcap & PPC_FEATURE_ARCH_2_05) + if (ppc_hwcap & PPC_FEATURE_HAS_DFP) init_registers_powerpc_isa205_vsx32l (); else init_registers_powerpc_vsx32l (); } else if (ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC) { - if (ppc_hwcap & PPC_FEATURE_ARCH_2_05) + if (ppc_hwcap & PPC_FEATURE_HAS_DFP) init_registers_powerpc_isa205_altivec32l (); else init_registers_powerpc_altivec32l (); Index: src/gdb/ppc-linux-nat.c =================================================================== --- src.orig/gdb/ppc-linux-nat.c 2009-03-04 10:46:06.000000000 -0300 +++ src/gdb/ppc-linux-nat.c 2009-03-25 18:11:00.000000000 -0300 @@ -63,8 +63,8 @@ #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 */ +#ifndef PPC_FEATURE_HAS_DFP +#define PPC_FEATURE_HAS_DFP 0x00000400 /* Decimal Floating Point. */ #endif /* Glibc's headers don't define PTRACE_GETVRREGS so we cannot use a @@ -1290,7 +1290,13 @@ ppc_linux_read_description (struct targe perror_with_name (_("Unable to fetch AltiVec registers")); } - if (ppc_linux_get_hwcap () & PPC_FEATURE_ARCH_2_05) + /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases + the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this + ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only + PPC_FEATURE_ARCH_2_06. Since for now the only bits used in the higher + half of the register are for Decimal Floating Point, we check if that + feature is available to decide the size of the FPSCR. */ + if (ppc_linux_get_hwcap () & PPC_FEATURE_HAS_DFP) isa205 = 1; /* Check for 64-bit inferior process. This is the case when the host is