From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30751 invoked by alias); 21 Oct 2003 22:22:50 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 30725 invoked from network); 21 Oct 2003 22:22:49 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 21 Oct 2003 22:22:49 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id h9LMMlM01682 for ; Tue, 21 Oct 2003 18:22:47 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h9LMMjr23265; Tue, 21 Oct 2003 18:22:45 -0400 Received: from localhost.localdomain (vpn50-5.rdu.redhat.com [172.16.50.5]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id h9LMMiCo031420; Tue, 21 Oct 2003 18:22:44 -0400 Received: (from kev@localhost) by localhost.localdomain (8.11.6/8.11.6) id h9LMMdu26262; Tue, 21 Oct 2003 15:22:39 -0700 Date: Tue, 21 Oct 2003 22:22:00 -0000 From: Kevin Buettner Message-Id: <1031021222239.ZM26261@localhost.localdomain> In-Reply-To: "J. Johnston" "Re: RFA: ia64 tdep patch" (Oct 20, 5:55pm) References: <3F9049EF.8060209@redhat.com> <1031020201315.ZM20659@localhost.localdomain> <3F9459B6.5000909@redhat.com> To: "J. Johnston" , Kevin Buettner Subject: Re: RFA: ia64 tdep patch Cc: gdb-patches@sources.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2003-10/txt/msg00647.txt.bz2 On Oct 20, 5:55pm, J. Johnston wrote: > 2003-10-20 Jeff Johnston > > * ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field. [...] > (ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get > sp needed for calling lower level The entry for ia64_sigtramp_frame_init_saved_regs() doesn't describe what was done there. AFAICT, the bumping by 16 part of the patch has been removed. You still do the following though: @@ -1869,10 +1913,8 @@ ia64_sigtramp_frame_init_saved_regs (str SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_CFM_REGNUM); cache->saved_regs[IA64_PSR_REGNUM] = SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_PSR_REGNUM); -#if 0 cache->saved_regs[IA64_BSP_REGNUM] = - SIGCONTEXT_REGISTER_ADDRESS (frame->frame, IA64_BSP_REGNUM); -#endif + SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_BSP_REGNUM); cache->saved_regs[IA64_RNAT_REGNUM] = SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_RNAT_REGNUM); cache->saved_regs[IA64_CCV_REGNUM] = @@ -1886,9 +1928,8 @@ ia64_sigtramp_frame_init_saved_regs (str cache->saved_regs[IA64_LC_REGNUM] = SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_LC_REGNUM); for (regno = IA64_GR1_REGNUM; regno <= IA64_GR31_REGNUM; regno++) - if (regno != sp_regnum) - cache->saved_regs[regno] = - SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno); + cache->saved_regs[regno] = + SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno); for (regno = IA64_BR0_REGNUM; regno <= IA64_BR7_REGNUM; regno++) cache->saved_regs[regno] = SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno); ...... The code (below) in ia64_sigtramp_frame_prev_register() which computes PSR doesn't look right to me. Could you check it? (If it is right, please explain it...) + else if (regnum == IA64_PSR_REGNUM) + { + ULONGEST slot_num = 0; + CORE_ADDR pc= 0; + CORE_ADDR psr = 0; + CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM]; + + if (addr != 0) + { + *lvalp = lval_memory; + *addrp = addr; + read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM)); + pc = extract_unsigned_integer (buf, 8); + } + psr &= ~(3LL << 41); + slot_num = pc & 0x3LL; + psr |= (CORE_ADDR)slot_num << 41; + store_unsigned_integer (valuep, 8, psr); + } ...... Regarding this hunk of code in ia64_sigtramp_frame_prev_register()... + else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) || + (regnum >= V32_REGNUM && regnum <= V127_REGNUM)) + { + CORE_ADDR addr = 0; + if (regnum >= V32_REGNUM) + regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM); + addr = cache->saved_regs[regnum]; + if (addr != 0) + { + *lvalp = lval_memory; + *addrp = addr; + read_memory (addr, valuep, register_size (current_gdbarch, regnum)); + } + } Could you add a comment explaining why the normal method of computing V32 (via ia64_pseudo_register_read()) is inadequate? Also, I'd prefer to see the following line: + if (regnum >= V32_REGNUM) written as + if (regnum >= V32_REGNUM && regnum <= V127_REGNUM) Hmm... doesn't this hunk of code also need to be concerned with register renames? (I.e, the rotating register stuff...) I'm wondering why the floating point registers need it, but the GRs don't. ...... Regarding... + else + { + CORE_ADDR addr = 0; + if (IA64_FR32_REGNUM <= regnum && regnum <= IA64_FR127_REGNUM) + { + /* Fetch floating point register rename base from current + frame marker for this frame. */ + int rrb_fr = (cache->cfm >> 25) & 0x7f; + + /* Adjust the floating point register number to account for + register rotation. */ + regnum = IA64_FR32_REGNUM + + ((regnum - IA64_FR32_REGNUM) + rrb_fr) % 96; + } + + /* If we have stored a memory address, access the register. */ + addr = cache->saved_regs[regnum]; + if (addr != 0) + { + *lvalp = lval_memory; + *addrp = addr; + read_memory (addr, valuep, register_size (current_gdbarch, regnum)); + } + } ...could you add a comment at the top of the block which says that it's intended to handle all the other registers (not handled by the previous clauses), floating point included. When I first looked at this, I saw the floating point register rename stuff and figured it was for just floating point. Kevin