From patchwork Tue Oct 18 16:35:57 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 16624 Received: (qmail 118749 invoked by alias); 18 Oct 2016 17:37:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 118736 invoked by uid 89); 18 Oct 2016 17:37:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: Yes, score=5.1 required=5.0 tests=BAYES_00, GARBLED_SUBJECT, KAM_ASCII_DIVIDERS, RP_MATCHES_RCVD, SPF_PASS autolearn=no version=3.3.2 spammy=restart, doub, restructuring, Missing X-HELO: mailapp01.imgtec.com Received: from mailapp02.imgtec.com (HELO mailapp01.imgtec.com) (217.156.133.132) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Oct 2016 17:37:02 +0000 Received: from HHMAIL03.hh.imgtec.org (unknown [10.44.0.21]) by Forcepoint Email with ESMTPS id 989729CEC3861 for ; Tue, 18 Oct 2016 18:36:56 +0100 (IST) Received: from HHMAIL01.hh.imgtec.org (10.100.10.19) by HHMAIL03.hh.imgtec.org (10.44.0.21) with Microsoft SMTP Server (TLS) id 14.3.294.0; Tue, 18 Oct 2016 18:36:59 +0100 Received: from [10.20.78.148] (10.20.78.148) by HHMAIL01.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server id 14.3.294.0; Tue, 18 Oct 2016 18:36:58 +0100 MIME-Version: 1.0 Date: Tue, 18 Oct 2016 17:35:57 +0100 From: "Maciej W. Rozycki" To: Bhushan Attarde CC: , Matthew Fortune , James Hogan , "Andrew Bennett" , Subject: Re: [PATCH 01/24] MIPS: Handle run-time reconfigurable FPR size In-Reply-To: Message-ID: References: <1467038991-6600-1-git-send-email-bhushan.attarde@imgtec.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) Resent-Date: Tue, 18 Oct 2016 18:36:48 +0100 Resent-From: "Maciej W. Rozycki" Resent-To: ReSent-Subject: Re: [PATCH 01/24] MIPS: Handle run-time reconfigurable FPR size Resent-Message-ID: ReSent-User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) Bhushan, On Mon, 25 Jul 2016, Maciej W. Rozycki wrote: > > @@ -8869,7 +8949,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > > mips_register_g_packet_guesses (gdbarch); > > > > /* Hook in OS ABI-specific overrides, if they have been registered. */ > > - info.tdep_info = tdesc_data; > > + ((struct gdbarch_tdep_info*)(info.tdep_info))->tdesc_data = tdesc_data; > > Missing space after the cast again, but why don't you need the same cast > to access `info.tdep_info->fp_register_size' above? Have you verified > this change actually builds? > > Overall again I think it'll be best refactored to avoid these casts, e.g. > rename the existing null `tdep_info' to `null_tdep_info', and then add a > new `tdep_info' variable to keep a correctly typed pointer and use it > throughout. So to address my own concerns I have now proposed `struct gdbarch_info' restructuring, so that none of the casts are actually needed, and neither are auxiliary variables. I forgot to cc you on that submission (sorry!); see here: . With that in place and once you've resolved the obvious merge conflict you can use the following update to the original "MIPS: Handle run-time reconfigurable FPR size" change, which is what I've been using. This also addresses my other concerns I have expressed in the review. Maciej Index: binutils/gdb/mips-linux-tdep.c =================================================================== --- binutils.orig/gdb/mips-linux-tdep.c 2016-10-18 07:51:21.000000000 +0100 +++ binutils/gdb/mips-linux-tdep.c 2016-10-18 07:55:18.734641371 +0100 @@ -1646,7 +1646,6 @@ mips_linux_init_abi (struct gdbarch_info { struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); enum mips_abi abi = mips_abi (gdbarch); - struct tdesc_arch_data *tdesc_data = info.tdesc_data; linux_init_abi (info, gdbarch); @@ -1738,7 +1737,7 @@ mips_linux_init_abi (struct gdbarch_info tdep->syscall_next_pc = mips_linux_syscall_next_pc; tdep->fp_register_size_fixed_p = 1; - if (((struct gdbarch_tdep_info*)(info.tdep_info))->tdesc_data) + if (info.tdep_info->tdesc_data) { const struct tdesc_feature *feature; @@ -1753,7 +1752,7 @@ mips_linux_init_abi (struct gdbarch_info feature = tdesc_find_feature (info.target_desc, "org.gnu.gdb.mips.linux"); if (feature != NULL) - tdesc_numbered_register (feature, (((struct gdbarch_tdep_info*)(info.tdep_info))->tdesc_data), + tdesc_numbered_register (feature, info.tdep_info->tdesc_data, MIPS_RESTART_REGNUM, "restart"); } } Index: binutils/gdb/mips-tdep.c =================================================================== --- binutils.orig/gdb/mips-tdep.c 2016-10-18 07:51:21.000000000 +0100 +++ binutils/gdb/mips-tdep.c 2016-10-18 17:33:42.662895228 +0100 @@ -6233,7 +6233,7 @@ mips_read_fp_register_single (struct fra { struct gdbarch *gdbarch = get_frame_arch (frame); int fpsize = register_size (gdbarch, regno); - gdb_byte *raw_buffer = alloca (fpsize); + gdb_byte *raw_buffer = (gdb_byte *) alloca (fpsize); if (!deprecated_frame_register_read (frame, regno, raw_buffer)) error (_("can't read register %d (%s)"), @@ -6306,12 +6306,10 @@ mips_print_fp_register (struct ui_file * { /* Do values for FP (float) regs. */ struct gdbarch *gdbarch = get_frame_arch (frame); int fpsize = register_size (gdbarch, regnum); - gdb_byte *raw_buffer; + gdb_byte *raw_buffer = (gdb_byte *) alloca (2 * fpsize); double doub, flt1; /* Doubles extracted from raw hex data. */ int inv1, inv2; - raw_buffer = alloca (2 * fpsize); - fprintf_filtered (file, "%s:", gdbarch_register_name (gdbarch, regnum)); fprintf_filtered (file, "%*s", 4 - (int) strlen (gdbarch_register_name (gdbarch, regnum)), @@ -6462,7 +6460,7 @@ mips_print_float_info (struct gdbarch *g return; fprintf_filtered (file, "reg size: %d bits\n", - register_size (gdbarch, mips_regnum (gdbarch)->fp0) * 8); + mips_float_regsize (gdbarch) * 8); fputs_filtered ("cond :", file); if (fcs & (1 << 23)) @@ -8930,7 +8928,7 @@ mips_gdbarch_init (struct gdbarch_info i mips_register_g_packet_guesses (gdbarch); /* Hook in OS ABI-specific overrides, if they have been registered. */ - ((struct gdbarch_tdep_info*)(info.tdep_info))->tdesc_data = tdesc_data; + info.tdep_info->tdesc_data = tdesc_data; gdbarch_init_osabi (info, gdbarch); /* The hook may have adjusted num_regs, fetch the final value and