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

[Mark Kettenis <kettenis at chello dot nl>]

   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:

   +	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?

   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.

   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?

   - 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?

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

Mark