Re: [RFC 00/15] Remove regcache::m_readonly_p

Yao Qi
2018-01-16T16:18:20Z

Yao Qi <qiyaoltc@gmail.com> writes:

> regcache is used in many places in gdb in different ways, so regcache
> becomes a flat and fat object. That exposes some unnecessary APIs to
> different part, and some APIs are misused or abused:
>
> 1) gdbarch methods pseudo_register_read and pseudo_register_read_value
> have a parameter 'regcache *', but these two gdbarch methods only need
> raw_read* and cooked_read* methods. So it is better to pass a class
> which only has raw_read* and cooked_read* methods, and other regcache
> methods are invisible to each gdbarch implementation.
>
> 2) target_ops methods to_fetch_registers and to_store_registers have a
> parameter 'regcache *', but these two target_ops methods only need
> raw_supply and raw_collect methods, because raw registers are from target
> layer, pseudo registers are "composed" or "created" by gdbarch.
>
> 3) jit.c uses regcache in an odd way, and record-full.c should use
> a simple version regcache instead of an array (see patch 11)
>
> Beside these api issues, one issue in regcache is that there is no
> type or class for readonly regcache. We use a flag field m_readonly_p
> to indicate the regcache is readonly or not, so some regcache apis have
> assert that this regcache is or is not readonly. The better way to do
> this is to create a new class for readonly regcache which doesn't have
> write methods at all.
>
> This patch series fixes all of the problems above except 2) (I had a
> patch to fix 2 in my tree, but still need more time to polish it.) by
> designing a class hierarchy about regcache, like this,
>
> reg_buffer
> ^
> |
> ------+-----
> ^
> |
> regcache_read
> ^
> |
> ------+------
> ^ ^
> | |
> reg_buffer_rw regcache_readonly
> ^
> |
> regcache
>
> Class reg_buffer is a simple class, having register contents and status
> (in patch 7). regcache_read is an abstract class only having raw_read*
> and cooked_read* methods (in patch 8). reg_buffer_rw is a class which
> has read and write methods, but it disconnects from target, IOW, the
> write doesn't go through. Class regcache_readonly is the readonly
> regcache, created from regcache::save method.
>
> This patch series is tested on {x86_64, aarch64, ppc64}-linux. It is
> an RFC, want to get comments. I write them in at least seven different
> ways, and this one is satisfying. I don't push them in until 8.1 is
> branched.

Ping. I'd like to know what do you think of this design, patches 7/15
to 15/15.

--
Yao (齐尧)