Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@imgtec.com>
To: Bhushan Attarde <bhushan.attarde@imgtec.com>
Cc: <gdb-patches@sourceware.org>,
	Matthew Fortune	<Matthew.Fortune@imgtec.com>,
	James Hogan <James.Hogan@imgtec.com>,
	"Andrew Bennett" <Andrew.Bennett@imgtec.com>,
	<Jaydeep.Patil@imgtec.com>
Subject: Re: [PATCH 01/24]     MIPS: Handle run-time reconfigurable FPR size
Date: Tue, 18 Oct 2016 17:37:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1610181721320.31859@tp.orcam.me.uk> (raw)
In-Reply-To: <alpine.DEB.2.00.1607221542560.4076@tp.orcam.me.uk>

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: <https://sourceware.org/ml/gdb-patches/2016-10/msg00516.html>. 

 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


  reply	other threads:[~2016-10-18 17:37 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-27 14:50 Bhushan Attarde
2016-06-27 14:50 ` [PATCH 03/24] regcache: handle invalidated regcache Bhushan Attarde
2016-10-21 22:42   ` Maciej W. Rozycki
2016-06-27 14:50 ` [PATCH 08/24] MIPS: Convert FP mode to enum and put fp registers into fp reggroup Bhushan Attarde
2016-06-27 14:50 ` [PATCH 10/24] MIPS: override fscr/fir types and print control registers specially Bhushan Attarde
2016-06-27 14:50 ` [PATCH 11/24] MIPS: Add support for hybrid fp32/fp64 mode Bhushan Attarde
2016-06-27 14:50 ` [PATCH 06/24] mips-linux-nat: pick fp64 target description when appropriate Bhushan Attarde
2016-06-27 14:51 ` [PATCH 21/24] MIPSR6 support for GDB Bhushan Attarde
2016-07-29 21:10   ` Maciej W. Rozycki
2016-06-27 14:51 ` [PATCH 23/24] MIPS R6 opcode table shuffle for LDC2/SDC2 Bhushan Attarde
2016-06-27 14:51 ` [PATCH 07/24] MIPS: Make Linux restart register more dynamic Bhushan Attarde
2016-06-27 14:51 ` [PATCH 13/24] Add MIPS MSA GDB target descriptions Bhushan Attarde
2016-06-27 14:51 ` [PATCH 18/24] mips-linux-nat: get msa registers Bhushan Attarde
2016-06-27 14:51 ` [PATCH 02/24] Add MIPS32 FPU64 GDB target descriptions Bhushan Attarde
2016-10-12 12:42   ` Maciej W. Rozycki
2016-10-12 13:58     ` James Hogan
2016-10-12 16:30       ` Maciej W. Rozycki
2016-10-12 18:05         ` James Hogan
2016-10-12 22:04           ` Maciej W. Rozycki
2016-10-13 10:09             ` Matthew Fortune
2016-10-21 19:17               ` Maciej W. Rozycki
2016-10-21 19:24                 ` Maciej W. Rozycki
2016-06-27 14:51 ` [PATCH 04/24] Add MIPS Config5 register related support Bhushan Attarde
2016-06-27 14:51 ` [PATCH 22/24] Support all new ABIs when detecting if an FPU is present Bhushan Attarde
2016-06-27 14:51 ` [PATCH 19/24] Add MIPS MSA vector branch instruction support Bhushan Attarde
2016-06-27 14:51 ` [PATCH 09/24] MIPS: Enhance cooked FP format Bhushan Attarde
2016-06-27 14:51 ` [PATCH 14/24] Implement core MSA stuff Bhushan Attarde
2016-06-27 14:51 ` [PATCH 20/24] Drop FP and MSA control registers from default info registers Bhushan Attarde
2016-06-27 14:51 ` [PATCH 12/24] o32 sigframe unwinding with FR1 Bhushan Attarde
2016-06-27 14:51 ` [PATCH 05/24] MIPS: Add config5 to MIPS GDB target descriptions Bhushan Attarde
2016-06-27 14:51 ` [PATCH 24/24] MIPS R6 forbidden slot support Bhushan Attarde
2016-07-25 14:03 ` [PATCH 01/24] MIPS: Handle run-time reconfigurable FPR size Maciej W. Rozycki
2016-10-18 17:37   ` Maciej W. Rozycki [this message]
2016-11-08 19:46 ` Yao Qi
2016-11-10 12:43   ` Maciej W. Rozycki
2016-11-11 12:29     ` Yao Qi
2016-12-02  2:31       ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.00.1610181721320.31859@tp.orcam.me.uk \
    --to=macro@imgtec.com \
    --cc=Andrew.Bennett@imgtec.com \
    --cc=James.Hogan@imgtec.com \
    --cc=Jaydeep.Patil@imgtec.com \
    --cc=Matthew.Fortune@imgtec.com \
    --cc=bhushan.attarde@imgtec.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox