From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9830 invoked by alias); 7 Jun 2013 16:40:13 -0000 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 Received: (qmail 9817 invoked by uid 89); 7 Jun 2013 16:40:13 -0000 X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_MED,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,TW_EG autolearn=ham version=3.3.1 Received: from e06smtp15.uk.ibm.com (HELO e06smtp15.uk.ibm.com) (195.75.94.111) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 07 Jun 2013 16:40:12 +0000 Received: from /spool/local by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 7 Jun 2013 17:36:03 +0100 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp15.uk.ibm.com (192.168.101.145) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 7 Jun 2013 17:36:01 +0100 Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 235281B08061 for ; Fri, 7 Jun 2013 17:40:08 +0100 (BST) Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by b06cxnps4074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r57GdvSk58065126 for ; Fri, 7 Jun 2013 16:39:57 GMT Received: from d06av02.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r57Ge63C017210 for ; Fri, 7 Jun 2013 10:40:07 -0600 Received: from br87z6lw.de.ibm.com (dyn-9-152-212-143.boeblingen.de.ibm.com [9.152.212.143]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id r57Ge6Xo017207; Fri, 7 Jun 2013 10:40:06 -0600 From: Andreas Arnez To: lgustavo@codesourcery.com Cc: gdb-patches@sourceware.org, Ulrich Weigand Subject: Re: [PATCH 3/3] Dynamic core regset sections support References: <87fvwu5937.fsf@br87z6lw.de.ibm.com> <87li6mvxgv.fsf@br87z6lw.de.ibm.com> <51B1F195.70008@codesourcery.com> Date: Fri, 07 Jun 2013 16:40:00 -0000 In-Reply-To: <51B1F195.70008@codesourcery.com> (Luis Machado's message of "Fri, 07 Jun 2013 16:43:33 +0200") Message-ID: <87hah9ub6h.fsf@br87z6lw.de.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13060716-0342-0000-0000-00000541C77D X-SW-Source: 2013-06/txt/msg00167.txt.bz2 Luis Machado writes: > Just a general thought. If a core file section must not always be > generated, would it make more sense to use dynamic section information > for s390 instead of forcing all the other targets to use a mechanism > that is not required for them? The reason for removing the old interface was to avoid increasing the number of gdbarch interfaces for this. In addition, the change reduces the amount of target-dependent code for most targets, which I consider beneficial. > I'm not necessarily against this modification, but i was wondering > about other options. Sure, I'm open for suggestions. > Did you maybe forget to handle ppc64 core file sections in this > change? Phew, you're right. I guess we can use tdep->wordsize instead, like this (right?): if ((*cb) (".reg", 48 * tdep->wordsize, "general-purpose", cb_data)) It would be great if you could verify this change -- I'm providing an updated patch below. > Another general comment/suggestion. Removing static definitions of > core file register sets and replacing them with hardcoded arguments > for a pointer-based call to a callback function seems to > scramble/confuse things a little. > > It may be nicer to keep the static definitions around, but iterate > through them in the new iterator function. What do you think? I thought about something like that as well. However, my implementation attempt didn't result in less (or simpler) code. Updated patch for linux-tdep.c: --- a/gdb/ppc-linux-tdep.c +++ b/gdb/ppc-linux-tdep.c @@ -256,53 +256,26 @@ ppc_linux_return_value (struct gdbarch *gdbarch, struct value *function, readbuf, writebuf); } -static struct core_regset_section ppc_linux_vsx_regset_sections[] = -{ - { ".reg", 48 * 4, "general-purpose" }, - { ".reg2", 264, "floating-point" }, - { ".reg-ppc-vmx", 544, "ppc Altivec" }, - { ".reg-ppc-vsx", 256, "POWER7 VSX" }, - { NULL, 0} -}; - -static struct core_regset_section ppc_linux_vmx_regset_sections[] = -{ - { ".reg", 48 * 4, "general-purpose" }, - { ".reg2", 264, "floating-point" }, - { ".reg-ppc-vmx", 544, "ppc Altivec" }, - { NULL, 0} -}; - -static struct core_regset_section ppc_linux_fp_regset_sections[] = -{ - { ".reg", 48 * 4, "general-purpose" }, - { ".reg2", 264, "floating-point" }, - { NULL, 0} -}; - -static struct core_regset_section ppc64_linux_vsx_regset_sections[] = -{ - { ".reg", 48 * 8, "general-purpose" }, - { ".reg2", 264, "floating-point" }, - { ".reg-ppc-vmx", 544, "ppc Altivec" }, - { ".reg-ppc-vsx", 256, "POWER7 VSX" }, - { NULL, 0} -}; - -static struct core_regset_section ppc64_linux_vmx_regset_sections[] = -{ - { ".reg", 48 * 8, "general-purpose" }, - { ".reg2", 264, "floating-point" }, - { ".reg-ppc-vmx", 544, "ppc Altivec" }, - { NULL, 0} -}; - -static struct core_regset_section ppc64_linux_fp_regset_sections[] = +static void +ppc_linux_iterate_over_regset_sections (struct gdbarch *gdbarch, + iterate_over_regset_sections_cb *cb, + void *cb_data, + const struct regcache *regcache) { - { ".reg", 48 * 8, "general-purpose" }, - { ".reg2", 264, "floating-point" }, - { NULL, 0} -}; + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); + int have_vsx = tdep->ppc_vr0_regnum != -1; + int have_altivec = tdep->ppc_vsr0_upper_regnum != -1; + + if ((*cb) (".reg", 48 * tdep->wordsize, "general-purpose", cb_data)) + return; + if ((*cb) (".reg2", 264, "floating-point", cb_data)) + return; + if (have_altivec) + if ((*cb) (".reg-ppc-vmx", 544, "ppc Altivec", cb_data)) + return; + if (have_vsx) + (*cb) (".reg-ppc-vsx", 256, "POWER7 VSX", cb_data); +} /* PLT stub in executable. */ static struct ppc_insn_pattern powerpc32_plt_stub[] = @@ -1305,18 +1278,9 @@ ppc_linux_init_abi (struct gdbarch_info info, else set_gdbarch_gcore_bfd_target (gdbarch, "elf32-powerpc"); - /* Supported register sections. */ - if (tdesc_find_feature (info.target_desc, - "org.gnu.gdb.power.vsx")) - set_gdbarch_core_regset_sections (gdbarch, - ppc_linux_vsx_regset_sections); - else if (tdesc_find_feature (info.target_desc, - "org.gnu.gdb.power.altivec")) - set_gdbarch_core_regset_sections (gdbarch, - ppc_linux_vmx_regset_sections); - else - set_gdbarch_core_regset_sections (gdbarch, - ppc_linux_fp_regset_sections); + /* Iterator for supported register sections. */ + set_gdbarch_iterate_over_regset_sections + (gdbarch, ppc_linux_iterate_over_regset_sections); if (powerpc_so_ops.in_dynsym_resolve_code == NULL) { @@ -1360,18 +1324,9 @@ ppc_linux_init_abi (struct gdbarch_info info, else set_gdbarch_gcore_bfd_target (gdbarch, "elf64-powerpc"); - /* Supported register sections. */ - if (tdesc_find_feature (info.target_desc, - "org.gnu.gdb.power.vsx")) - set_gdbarch_core_regset_sections (gdbarch, - ppc64_linux_vsx_regset_sections); - else if (tdesc_find_feature (info.target_desc, - "org.gnu.gdb.power.altivec")) - set_gdbarch_core_regset_sections (gdbarch, - ppc64_linux_vmx_regset_sections); - else - set_gdbarch_core_regset_sections (gdbarch, - ppc64_linux_fp_regset_sections); + /* Iterator for supported register sections. */ + set_gdbarch_iterate_over_regset_sections + (gdbarch, ppc_linux_iterate_over_regset_sections); } /* PPC32 uses a different prpsinfo32 compared to most other Linux