public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Re:Re:[PATCH] gdb/csky complete csky_dwarf_reg_to_regnum
@ 2022-07-19  3:00 李江帅
  0 siblings, 0 replies; only message in thread
From: 李江帅 @ 2022-07-19  3:00 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches


>>>> +  if (dw_reg >= CSKY_R0_REGNUM && dw_reg <= (CSKY_R0_REGNUM + 31))
>>> 
>>> This line and some others have too many parentheses.
>
>> This line meens that "if (dw_reg >=0 && dw_reg <= 31)". As i does not define CSKY_R31_REGNUM,
>> i wrote 31 as (CSKY_R0_REGNUM + 31).
>
>Yeah, what I mean is that there doesn't seem to be a need for the parens
>in the second comparison.  This sort of thing is more the gdb style:
>
>  if (dw_reg >= CSKY_R0_REGNUM && dw_reg <= CSKY_R0_REGNUM + 31)

OK, Get. The priority of '+' is higher than '>=', but if we does not want to check it, add '()'
maybe a good choice.

>
>There are other cases like this:
>
>>>> +  /* For Float and Vector pseudo registers.  */
>>>> +  if ((dw_reg >= FV_PSEUDO_REGNO_FIRST)  && (dw_reg <= FV_PSEUDO_REGNO_LAST))
>
>... where there are excessive parens.

Get.

>
>>>> +      xsnprintf (name_buf, sizeof (name_buf), "s%d",
>>>> +                 dw_reg - FV_PSEUDO_REGNO_FIRST);
>>>> +      return user_reg_map_name_to_regnum (gdbarch, name_buf,
>>>> +                                          strlen (name_buf));
>>> 
>>> This seems like a very roundabout approach.  Is there a reason it's done
>>> this way?
>
>> I am not sure what you mean, this code mean that:
>> if (dw_reg >= 74 && dw_reg <= 201),  dw_reg will corresponds to a pseudo register
>> added for float or vector registers.
>
>> Similar code: arm-tdep.c: 4876
>
>What I mean here is that this code looks up the register by name to get
>its number.  But why is this extra step needed?  It's fine if it's
>needed, but I'd like to understand why.

There are two reasons i can think of:
1. When we add some pseudo registers, the number of these registers in GDB is immediately followed
 after by gdbarch_num_regs (gdbarch).  When GDB loads tdesc-xml, the result of gdbarch_num_regs (gdbarch)
 is variable, so the register number of pseudo is not fixed inside GDB.

2. The numbers of pseudo registers are bound to its corresponding read & write functions, which is a rule. 
When the rule is fixed, the numbers of pseudo registers are relatively fixed on the basis of gdbarch_num_regs (gdbarch) .
If this rule is modified, that is, the arrangement of pseudo registers changes, then the number of pseudo
registers will also changes. 

At this point, we can still use uses the current code in csky_dwarf_reg_to_regnum regardless of whether
 the gdbarch_num_regs (gdbarch) changes or the arrangement of pseudo registers changes.

Jiangshuai.


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-07-19  3:00 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19  3:00 Re:Re:[PATCH] gdb/csky complete csky_dwarf_reg_to_regnum 李江帅

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).