Re: [PATCH] [Arm] Remove dead FPA code

[Luis Machado <luis.machado@arm.com>]

On 10/10/22 15:56, Pedro Alves wrote:
> Hi,
> 
> On 2022-10-04 9:43 a.m., Luis Machado wrote:
>>
>>
>>>> -   The packet format which
>>>> -   includes FPA registers should be considered deprecated for
>>>> -   M-profile targets.  */
>>>> -
>>>> -static void
>>>> -arm_register_g_packet_guesses (struct gdbarch *gdbarch)
>>>> -{
>>>> -  arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
>>>> -
>>>> -  if (tdep->is_m)
>>>> -    {
>>>> -      const target_desc *tdesc;
>>>> -
>>>> -      /* If we know from the executable this is an M-profile target,
>>>> -     cater for remote targets whose register set layout is the
>>>> -     same as the FPA layout.  */
>>>> -      tdesc = arm_read_mprofile_description (ARM_M_TYPE_WITH_FPA);
>>>> -      register_remote_g_packet_guess (gdbarch,
>>>> -                      ARM_CORE_REGS_SIZE + ARM_FP_REGS_SIZE,
>>>> -                      tdesc);
>>>> -
>>>> -      /* The regular M-profile layout.  */
>>>> -      tdesc = arm_read_mprofile_description (ARM_M_TYPE_M_PROFILE);
>>>> -      register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE,
>>>> -                      tdesc);
>>>> -
>>>> -      /* M-profile plus M4F VFP.  */
>>>> -      tdesc = arm_read_mprofile_description (ARM_M_TYPE_VFP_D16);
>>>> -      register_remote_g_packet_guess (gdbarch,
>>>> -                      ARM_CORE_REGS_SIZE + ARM_VFP2_REGS_SIZE,
>>>> -                      tdesc);
>>>> -      /* M-profile plus MVE.  */
>>>> -      tdesc = arm_read_mprofile_description (ARM_M_TYPE_MVE);
>>>> -      register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE
>>>> -                      + ARM_VFP2_REGS_SIZE
>>>> -                      + ARM_INT_REGISTER_SIZE, tdesc);
>>>> -
>>>> -      /* M-profile system (stack pointers).  */
>>>> -      tdesc = arm_read_mprofile_description (ARM_M_TYPE_SYSTEM);
>>>> -      register_remote_g_packet_guess (gdbarch, 2 * ARM_INT_REGISTER_SIZE, tdesc);
>>>
>>> And what about all these other non-FPA guesses?  Do you really mean to drop them?
>>>
>>
>> More recently we've added two new guesses for MVE and M-profile system registers, but it doesn't
>> make sense to do so, as these features are advertised as XML already.
> 
> What was the justification for adding them back then, then?
> 

It was meant to support other register sets under a situation where no XML is sent back. Though valid, my feeling is that
it was done mostly as copy/pasting what was already being done for the GPR's/FPA. The documentation isn't exactly clear
on whether one should do it or not. Only a few targets use these guesses (Arm, Mips and Microblaze).

I should probably add something to that effect in the future.

>> So in my opinion, we should
>> drop the g packet guesses completely and rely solely on XML descriptions.
> 
> Then I think that it should be proposed in a separate patch, as it has nothing to
> do with dropping FPA support.

Makes sense. I don't think we should be adding any other guesses for newer features/regsets.
> 
> Off hand, I don't really see what you win much removing the guesses.  The code that it allows
> removing isn't something that would block any redesign or big cleanup in gdb, as gdb needs to know
> how to create built-in tdescs for core files, anyhow (even if not these particular tdescs),
> and the tdesc generation code is even shared with gdbserver.

Well, based on recent discussions, we're not going to drop this, as Linux/BSD kgdb is still relying on it.

Removing the guesses doesn't buy us much on its own, but there is opportunity to get targets to move
to XML descriptions, which is something easier to work with, more flexible and less error-prone. So it looks
like a step in the right direction.