Re: [cagney_convert-20030606-branch] Add value to REGISTER_TO_VALUE et.al.

[Andrew Cagney <ac131313@redhat.com>]

>    Date: Fri, 06 Jun 2003 14:12:23 -0400
>    From: Andrew Cagney <ac131313@redhat.com>
> 
>    Mark,
> 
>    I've created the branch: cagney_convert-20030606-branch and committed 
>    the attached.  It's different from the original in the following ways:
> 
>    - REGISTER_TO_VALUE and VALUE_TO_REGISTER take the full ``struct value'' 
>    instead of ``struct type'' and a ``void *buf''.  Those methods are now 
>    entirely responsible for converting a register to/from the specified value.
> 
> There is defenitely something wrong here.  The first thing is that in
> valops.c:value_assign() you call CONVERT_REGISTER_P as follows:

I'm not suprised, nothing in the testsuite was testing it.

>    +	if (CONVERT_REGISTER_P (VALUE_REGNO (toval), VALUE_TYPE (toval)))
> 
> This is wrong for the lval_reg_frame_relative case.  Replacing
> `VALUE_REGNO (toval)' with `value_reg' seems the obvious solution, and
> indeed, this makes us execute the following line:
> 
>    +	    VALUE_TO_REGISTER (frame, fromval);
> 
> This is strange.  Which register should FROMVAL be stored in?  That
> information is contained in TOVAL.  Passing TOVAL here is also wrong,
> since the contents of the value are contained in FROMVAL.  A possible
> solution is passing the register number to VALUE_TO_REGISTER.  Another
> solution is assigning the contents of FROMVAL to (a copy of) TOVAL and
> pass the latter to VALUE_TO_REGISTER.  This seems to be what you had
> in mind when you wrote the MIPS implementation of VALUE_TO_REGISTER.
> 
> Besides this issue I have a few other comments on these interfaces.
> 
> 1. I think it is a good idea to ask the following question again: Why
>    do we have both lval_register and lval_reg_frame_relative?

lval_memory and lval_register are important when it comes to storing 
registers, but not when tracking variables.  A variable should really 
only have lval_reg_frame_relative.  Tried it though and things ``broke''.

>    Both seem to deal with values stored in registers.  Yes, these can
>    be frame-relative or not.  But wouldn't it be better to let
>    VALUE_FRAME_ID() indicate this?  A value of null_frame_id would
>    then indicate a global register variable.

Yep, ref: Use of lval_register?
http://sources.redhat.com/ml/gdb/2003-06/msg00065.html
In theory, it's always frame-relative.

>    I may be mistaken, but I think this has consequences for the
>    REGISTER_TO_VALUE interface.  Instead of an interface that
>    constructs the complete value, I think we need an interface that
>    just fetches the contents from the appropriate registers; something
>    that is similar to what we have now.
> 
> 2. What do we guarantee about the `struct value' that is passed to
>    VALUE_TO_REGISTER?  That it is created by REGISTER_TO_VALUE?

The code guarentee that REGISTER_CONVERT_P holds and, by implication, 
REGISTER_TO_VALUE was used to construct it.

> 
>    - Adds the possibly contraversal put_frame_register method.  It turned 
>    out that at least three functions contained the code sequence: find 
>    REGNUM's true location, store the value in that location.
> 
> Nothing against the function, but can we try to keep the naming of the
> register functions a bit more symmetrical?

I do.  Its the frame oposites (get_frame_xxxxx) that are the problem and 
need some savage renaming.

> Anyway, I don't think you should check this in just yet.  It
> definitely needs some more discussion.

I was assuming you were going to fix it (or at least the write side :-)

The ``obvious'' interfaces were:

register_to_value (frame, regnum, type, buffer)
value_to_register (frame, regnum, type, buffer)

but that tripped up on something (now what ...?).  Dig dig.  Notice how, 
to preserve existing behavior, legacy_register_to_value saves the 
location based on what frame_register returns.  We'd have to switch to 
lval_reg_frame_relative.

Andrew