Re：[PATCH] gdb/csky complete csky_dwarf_reg_to_regnum

Re：[PATCH] gdb/csky complete csky_dwarf_reg_to_regnum

[李江帅]

>>>>>> Jiangshuai Li <jiangshuai_li@c-sky.com> writes:

>
>> +  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).

>> +  /* For Float and Vector pseudo registers.  */
>> +  if ((dw_reg >= FV_PSEUDO_REGNO_FIRST)  && (dw_reg <= FV_PSEUDO_REGNO_LAST))
>> +    {
>> +      char name_buf[4];
>> +
>> +      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

>
>Tom

Jiangshuai.

Re: Re：[PATCH] gdb/csky complete csky_dwarf_reg_to_regnum

[Tom Tromey]

>>> +  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)

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.

>>> +      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.

Tom