Re: [PATCH 1/7] Regcache: Subclass ptid functionality to target_regcache

Re: [PATCH 1/7] Regcache: Subclass ptid functionality to target_regcache

[Yao Qi]

Alan Hayward <Alan.Hayward@arm.com> writes:

> All ptid related functions are moved to target_regcache.
>

What is the rationale of this change?  The regcache is per-thread, even
it is disconnected from target.

> A regcache retains the ptid () method, which always returns -1.

ptid_t (-1) is minus_one_ptid, which has a special meaning.

> This ensures users can always test if a regcache is attached to a
> target, without needing to know if it is a target_regcache.

When do we need such test ("if a regcache is attached to a target")?

-- 
Yao (齐尧)

Re: [PATCH 1/7] Regcache: Subclass ptid functionality to target_regcache

[Alan Hayward]

> On 23 Aug 2017, at 11:33, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
>> All ptid related functions are moved to target_regcache.
>> 
> 
> What is the rationale of this change?  The regcache is per-thread, even
> it is disconnected from target.

In the existing code, when calling regcache_dup / copy constructor,
ptid of the new recache is always set to -1.

In the save / restore functions the ptid is not updated.

The regcache.c functions which read/write the ptid only do that using the
target connected regcaches.

Calling regcache_get_ptid on a readonly regcache will result in an assert
firing.


Therefore, in existing code, a readonly recache will always have a ptid
of -1. In the new code this property now becomes part of a detached
regcache.

However.....

> 
>> A regcache retains the ptid () method, which always returns -1.
> 
> ptid_t (-1) is minus_one_ptid, which has a special meaning.

Agreed. The existing code already treats -1 tpid to mean different things.

I’ve been thinking about this again.
regcache_get_ptid asserts if ptid is -1. Therefore ptid() should also
assert on a detached recache ?
With this change, a detached recache would never have a ptid.
I think that simplifies the code too.

> 
>> This ensures users can always test if a regcache is attached to a
>> target, without needing to know if it is a target_regcache.
> 
> When do we need such test ("if a regcache is attached to a target”)?

We don’t. I’m happy to drop this statement.
If we do need to add a test, there will probably be a better way of doing it.

> 
> -- 
> Yao (齐尧)