Re: [PATCH 1/2] gdb/alpha: Add target description support

[Simon Marchi <simark@simark.ca>]

On 2025-06-12 15:29, Yodel Eldar wrote:
> 
> On 6/4/25 12:29 PM, Yodel Eldar wrote:
>>
>> On 6/4/25 9:49 AM, Maciej W. Rozycki wrote:
>>> On Wed, 4 Jun 2025, Yodel Eldar wrote:
>>>
>>>>> Do we need to call tdesc_numbered_register for the register whose name
>>>>> is ""?  I suppose that we do, for backwards compatibility, when
>>>>> debugging against a remote target that doesn't send a target
>>>>> description?
>>>> Hi Simon and thanks for your time and question! I think we do, because
>>>> (IIUC) suppose an older remote target responds to, say, a 'p n' packet,
>>>> but the anonymous register is unaccounted for by the client, wouldn't
>>>> that break compatibility? Can't say for certain, so I defer to your and
>>>> the community's wisdom and err on the side of caution as I investigate
>>>> it.
>>>   Are there any remote Alpha targets there in the first place?
>>>
>>>   I only have writing Alpha/Linux gdbserver on my todo list once I'm done
>>> with the more urgent Alpha stuff (Linux kernel EV4 support restoration,
>>> GCC LRA conversion) and I haven't heard of any other Alpha GDB RSP debug
>>> stub either.  There *might* be something in QEMU; I guess that'd be the
>>> only place to check.
>>>
>>>    Maciej
>>>
>>
>> Maciej, you hit the nail on the head! Indeed, QEMU is the project I
>> alluded to in the cover letter for the patch, and I have been using it
>> for testing throughout the development of the patch. It also provided
>> the impetus to write the target description for Alpha in the first
>> place as it uses them for its execlog plugin.
>> Thanks for taking the time to reply despite having more pressing
>> Alpha tasks. I will keep an eye out for your forthcoming submissions,
>> and provide my modest feedback if it's helpful.
>>
>> Yodel
>>
> 
> Hi Simon et al!
> 
> Update regarding the feasibility of skipping the tdesc_numbered_register
> (TNR) call on the anonymous register:
> 
> I modified the patch to skip the TNR call (see below for diff), and ran
> the "maintenance print remote-registers" command while connected to a
> QEMU Alpha remote with "set debug remote on" and observed the following
> (see below for the abridged command output):
>   1. GDB internally creates a 0-bit wide placeholder at the anonymous
>   register's expected index (65), and that placeholder shares the GDB
>   internal offset of the unique register (520).
>   2. GDB infers the anonymous register as its 67th register with
>   internal offset 528 and type int64_t.
>   3. Despite having an internal number of 67, GDB does assign the
>   anonymous register the correct remote number of 65 and the correct
>   g/G offset of 520.
>   4. 'g' and 'p n' packets (particularly, where n is unique's remote
>   number 66) do not appear to corrupt the regcache as I had feared.
> 
> On the one hand, observations 3 and 4 suggest that skipping TNR on the
> anonymous register could work, but the discrepancy between the internal
> versus g/G offsets and the internal versus remote numbers (observations
> 1 and 2) makes me wary of some hard-to-find side-effect that I may have
> yet overlooked. Furthermore, 67 is the number of total registers, so
> assigning 67 for the anonymous register's internal number (observation
> 3) could be problematic.
> 
> Moreover, having GDB implicitly account for the anonymous register at 67
> while also displaying a 0-bit register placeholder at 65 seems
> gratuitous and probably offsets any performance benefit derived from
> skipping TNR on the anonymous register.
> 
> With regard to backwards compatibility, I compiled a 2011 version of
> QEMU, commit b758aca1f6c, and performed the same experiment as above,
> and the output of the maintenance command was identical to the recent
> version used earlier. Likewise for the 2008 commit 19bf517b7f7 that
> introduced GDB stub support for QEMU's Alpha target, although this
> version had not implemented PALcode functionality and lacked the unique
> register. Fortunately, all three did not appear to have problems
> communicating with the GDB client, notwithstanding my concerns, but the
> scope of my testing was limited to the QEMU targets, and I don't think
> it qualifies as a comprehensive answer to the compatibility question as
> my inability to find other targets does not preclude their existence.
> 
> Thus, I recommend against the omission of tdesc_numbered_register on the
> anonymous register.
> 
> Please let me know how to proceed; should v2 of the patch omit the
> tdesc_numbered_register call on the anonymous register? I defer to your
> judgment. Thanks!

Thanks for the detailed investigation.  I'm not super familiar with
register descriptions and the register numbering.  I think it's better
to be on the safe side and leave it there, it shouldn't bother anybody.

From my non-Alpha-expert point of view, the series LGTM, so I'm tempted
to approve it.  Maciej, did you have some comments on the code?  Does it
look good to you?

Simon