Re: [PATCH v2 6/6] gdb/aarch64: Detect vector length changes when debugging remotely

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Luis Machado <luis.machado@arm.com> writes:

> On 11/26/22 02:04, Thiago Jung Bauermann wrote:
>> @@ -8068,6 +8078,21 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply,
>>         /* Expedited registers.  */
>>         if (!stop_reply->regcache.empty ())
>>   	{
>> +	  /* If GDB already knows about this thread, we can give the
>> +	     architecture-specific code a chance to update the gdbarch based on
>> +	     the expedited registers.  */
>> +	  if (find_thread_ptid (this, ptid) != nullptr)
>> +	    {
>> +	      stop_reply->arch = gdbarch_update_architecture (stop_reply->arch,
>> +							      stop_reply->regcache);
>> +
>> +	      /* Save stop_reply->arch so that it can be returned by the
>> +		 thread_architecture method.  */
>> +	      remote_thread_info *remote_thr = get_remote_thread_info (this,
>> +								       ptid);
>> +	      remote_thr->expedited_arch = stop_reply->arch;
>> +	    }
>> +
>>   	  struct regcache *regcache
>>   	    = get_thread_arch_regcache (this, ptid, stop_reply->arch);
>>   @@ -14382,6 +14407,23 @@ remote_target::thread_info_to_thread_handle (struct
>> thread_info *tp)
>>     return priv->thread_handle;
>>   }
>>   +struct gdbarch *
>> +remote_target::thread_architecture (ptid_t ptid)
>> +{
>> +  thread_info *thr = find_thread_ptid (this, ptid);
>> +  remote_thread_info *remote_thr = nullptr;
>> +
>> +  if (thr != nullptr)
>> +    remote_thr = get_remote_thread_info (thr);
>> +
>> +  if (remote_thr == nullptr || remote_thr->expedited_arch == nullptr)
>> +    /* The default thread_architecture implementation is the one from
>> +       process_stratum_target.  */
>> +    return process_stratum_target::thread_architecture(ptid);
>> +
>> +  return remote_thr->expedited_arch;
>> +}
>> +
>>   bool
>>   remote_target::can_async_p ()
>>   {
>
> Just recalled this. One deficiency of the current SVE implementation
> is that it doesn't have a proper way to hide the Z register when they
> don't have any meaningful state (SVE not active, so we have fpsimd
> state).

I see the SVE_HEADER_FLAG_SVE being checked/set in
aarch64_linux_supply_sve_regset and aarch64_linux_collect_sve_regset
(though in the latter it is set unconditionally), and also HAS_SVE_STATE
being used in aarch64_sve_regs_copy_to_reg_buf and
aarch64_sve_regs_copy_from_reg_buf.

But I wasn't able to find similar logic regarding hiding the Z
registers. Do you have any pointers?

> Given the VL will always be reported as > 0, even if there is no
> active SVE state, gdb will always attempt to show Z registers. And
> those are quite verbose.

The VG pseudo-register is defined with an int type, so we could return a
negative value to indicate that the Z registers are unused (and the
module of the value would still be the vector granule). Would that
be ok?

> In this sense, the implementations of the the thread_architecture
> methods for both native aarch64 and remote differ. While native
> aarch64's implementation is capable of detecting the lack of active
> SVE state, the remote implementation can't. If gdbserver decided to
> drop the Z registers mid-execution (before the SVE state is not
> active), there wouldn't be vg for gdb to make a decision.

Looking at aarch64_linux_nat_target::thread_architecture my impression
is that its result depends only on aarch64_sve_get_vq, so I don't see
how it's different from the remote implementation.

> I wonder if there is a better way to handle this that would still
> allow us to hide these scalable registers when there is no active
> state.

I guess it depends on whether you would consider using negative VG
values as better. :-)

-- 
Thiago