[PATCH 0/7] Regcache: Split out target regcache functionality

[PATCH 0/7] Regcache: Split out target regcache functionality

[Alan Hayward]

This set of patches splits the existing regcache into a regcache
and a target_regcache subclass.
Doing this will simplify the interactions between a regcache and
the target, and will allow for the writable regcache copies.

A target_regcache is a regcache connected to a target. Reads and
writes of register values are passed through to the target.
A target_regcache cannot be readonly (because this doesn't make
sense).

Meanwhile, a regcache (sometime referred to as a detached regcache)
is not connected to a target. By default it is readonly, but does
not have to be. In addition to the raw registers a regcache also
caches cooked register values. Duplicating a target_regcache will
always result in a detached regcache.

Both regcache and target_regcache support the full set of raw_read,
raw_write, raw_collect, raw_supply, cooked_read, cooked_write
functions and all their variations. A user of a regache does not
need to treat the two types any differently - your regcache just
does the correct thing. For example, on a target_regcache,
raw_write will write to both the cache and the target, but on
a recache it will only write to the cache.

With this set of patches, the regcache for the current thread (as
given by get_current_regcache()), is now a target_regcache, that
will perform exactly the same functionality as the regcache in
the exisiting head.

An earlier plan for this set of patches was that the detached
regcache would not support raw_write, raw_collect, cooked_read,
cooked_write. The problem is many of the target hooks then need
updating to only accept target_regcache. Many of the implementations
of the target hooks then call back into the regcache, causing those
functions in turn to be dependant on the new classes, exploding the
size of the patch. It then become very fiddly/confusing to maintain
which type of regcache is required at any point.

If you want to look at the while series together, I’ve pushed it to:
remotes/origin/users/alahay01/targetregcache

The whole set of patches have been tested on a --enable-targets=all
build with board files unix, native-gdbserver and unittest.exp.


Thanks,
Alan.

Re: [PATCH 0/7] Regcache: Split out target regcache functionality

[Yao Qi]

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

> A target_regcache is a regcache connected to a target. Reads and
> writes of register values are passed through to the target.
> A target_regcache cannot be readonly (because this doesn't make
> sense).

At this stage, can we don't assume the readonly-ness of target_regcache?
https://sourceware.org/ml/gdb-patches/2017-07/msg00252.html

>
> Meanwhile, a regcache (sometime referred to as a detached regcache)
> is not connected to a target. By default it is readonly, but does
> not have to be. In addition to the raw registers a regcache also
> caches cooked register values. Duplicating a target_regcache will
> always result in a detached regcache.
>
> Both regcache and target_regcache support the full set of raw_read,
> raw_write, raw_collect, raw_supply, cooked_read, cooked_write
> functions and all their variations. A user of a regache does not
> need to treat the two types any differently - your regcache just
> does the correct thing. For example, on a target_regcache,
> raw_write will write to both the cache and the target, but on
> a recache it will only write to the cache.
>
> With this set of patches, the regcache for the current thread (as
> given by get_current_regcache()), is now a target_regcache, that
> will perform exactly the same functionality as the regcache in
> the exisiting head.
>
> An earlier plan for this set of patches was that the detached
> regcache would not support raw_write, raw_collect, cooked_read,
> cooked_write. The problem is many of the target hooks then need

My suggestion was that detached regcache doesn't have
{raw,cooked}_{read,write}_ methods, only has collect and supply methods.
and attached one has {raw,cooked}_{read,write}_ methods.
https://sourceware.org/ml/gdb-patches/2017-07/msg00127.html

> updating to only accept target_regcache. Many of the implementations
> of the target hooks then call back into the regcache, causing those
> functions in turn to be dependant on the new classes, exploding the
> size of the patch. It then become very fiddly/confusing to maintain
> which type of regcache is required at any point.

target hooks should call regcache supply and collect methods, IMO.

-- 
Yao (齐尧)

Re: [PATCH 0/7] Regcache: Split out target regcache functionality

[Yao Qi]

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

> The whole set of patches have been tested on a --enable-targets=all
> build with board files unix, native-gdbserver and unittest.exp.

On x86_64-linux or aarch64-linux?  GDB testsuite may not cover your
changes, good to add some unit tests to regcache.

-- 
Yao (齐尧)

Re: [PATCH 0/7] Regcache: Split out target regcache functionality

[Alan Hayward]

> On 23 Aug 2017, at 11:02, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
>> A target_regcache is a regcache connected to a target. Reads and
>> writes of register values are passed through to the target.
>> A target_regcache cannot be readonly (because this doesn't make
>> sense).
> 
> At this stage, can we don't assume the readonly-ness of target_regcache?
> https://sourceware.org/ml/gdb-patches/2017-07/msg00252.html

Ok, I didn’t realise core files were treated as a target.
I need to look at the code more for this, but I couldn’t find anything in the
exisiting code that makes a core recache readonly.
If so, maybe it’s my code that needs to add this functionality.

> 
>> 
>> Meanwhile, a regcache (sometime referred to as a detached regcache)
>> is not connected to a target. By default it is readonly, but does
>> not have to be. In addition to the raw registers a regcache also
>> caches cooked register values. Duplicating a target_regcache will
>> always result in a detached regcache.
>> 
>> Both regcache and target_regcache support the full set of raw_read,
>> raw_write, raw_collect, raw_supply, cooked_read, cooked_write
>> functions and all their variations. A user of a regache does not
>> need to treat the two types any differently - your regcache just
>> does the correct thing. For example, on a target_regcache,
>> raw_write will write to both the cache and the target, but on
>> a recache it will only write to the cache.
>> 
>> With this set of patches, the regcache for the current thread (as
>> given by get_current_regcache()), is now a target_regcache, that
>> will perform exactly the same functionality as the regcache in
>> the exisiting head.
>> 
>> An earlier plan for this set of patches was that the detached
>> regcache would not support raw_write, raw_collect, cooked_read,
>> cooked_write. The problem is many of the target hooks then need
> 
> My suggestion was that detached regcache doesn't have
> {raw,cooked}_{read,write}_ methods, only has collect and supply methods.
> and attached one has {raw,cooked}_{read,write}_ methods.
> https://sourceware.org/ml/gdb-patches/2017-07/msg00127.html

Agreed. Typo in my description.

> 
>> updating to only accept target_regcache. Many of the implementations
>> of the target hooks then call back into the regcache, causing those
>> functions in turn to be dependant on the new classes, exploding the
>> size of the patch. It then become very fiddly/confusing to maintain
>> which type of regcache is required at any point.
> 
> target hooks should call regcache supply and collect methods, IMO.
> 

Agreed in principle, but this is would be a large change!
It could have hidden side effects - a target that calls cooked_read might be
expecting to read a pseudo reg.

I wanted to avoid that change in this patch series.

A later set of patches could change the targets to only call supply/collect
methods. Possibly one target at a time. Once this is fixed, then the additional
methods can be removed from detached regcaches.

> -- 
> Yao (齐尧)