This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Replace regcache readonly flag with detached flag
> 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.