Re: [RFC] Replace regcache readonly flag with detached flag

[Alan Hayward <Alan dot Hayward at arm dot com>]

> On 18 Jul 2017, at 10:47, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
> Hi Alan,
> 
>> In your regcache_1 constructor, you only NEW the cooked registers if the
>> regcache is readonly.
>> http://people.linaro.org/~yao.qi/gdb/doxy/regcache-split/gdb-xref/classregcache__1.html#acef3ef3bc85269cf04728901b4f28ee8
>> In my version I only NEW the cooked registers in a detached register cache.
> 
> We can change regcache_1 constructor if it is wrong.  I am not saying my
> patches are correct, let's commit them.  When I reviewed your patch, I
> thought it is better to split regcache, then, I spent one day writing
> the code to make sure the idea of splitting regcache makes sense.  I
> post the doxygen link rather than patches, because my code is still
> poor, but I think the doxygen is sufficient to show the design.

Understood.

> 
>> 
>> As I understand it, the cooked registers exist because on some architectures
>> extra state needs saving in the cooked registers (code comment: "some
>> architectures
>> need to save/restore `cooked registers that live in memory.”).
>> 
>> Therefore the cooked register state needs to be a property of detached
>> and not of
>> readonly.
>> 
> 
> m_registers and m_register_status are fields of detached regcache, we
> can definitely save cooked register state in detached regcache.
> 
>> 
>> A different issue is that we treat save/restore differently.
>> In your code one of the recaches has to be both read-only (checking
>> via gdb_assert) and detached.
>> In my code the check is that the regcache is detached or
>> not. Read-only is not relevant.
> 
> It is read-only in my code, but it doesn't have to be.  I don't see any
> show-stoppers in the design of splitting regcache.  The attributes
> "detached" and "read-only" are orthogonal in design.  Do you have some
> comments on the overall design rather than the code details?  I'll
> rewrite my patches, and post them.  It is unfortunate that it is hard to
> review the overall design without the code.

I think we’ve covered my main points:
* Use an detached bool instead of splitting the class. (Happy to forget this now)
* Making sure incorrect casting doesn’t happen. (Happy that this is ok)
* cooked state is a property of detached, not readonly.

I’m happy with the rest of the design.

When you post the diff, I’ll apply my record-full patch on top and make
sure it still works.


Alan.