Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Mark Kettenis <kettenis@chello.nl>
To: ac131313@redhat.com
Cc: gdb-patches@sources.redhat.com
Subject: Re: [cagney_convert-20030606-branch] Add value to REGISTER_TO_VALUE et.al.
Date: Sun, 08 Jun 2003 16:43:00 -0000	[thread overview]
Message-ID: <200306081643.h58GhiTr048427@elgar.kettenis.dyndns.org> (raw)
In-Reply-To: <3EE0D987.6030207@redhat.com> (message from Andrew Cagney on Fri, 06 Jun 2003 14:12:23 -0400)

   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


  reply	other threads:[~2003-06-08 16:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-06-04 19:38 [wip/rfc] Merge REGISTER_TO_VALUE and REGISTER_TO_TYPE Andrew Cagney
2003-06-04 21:45 ` Mark Kettenis
2003-06-04 23:05   ` Andrew Cagney
2003-06-06 18:12   ` [cagney_convert-20030606-branch] Add value to REGISTER_TO_VALUE et.al Andrew Cagney
2003-06-08 16:43     ` Mark Kettenis [this message]
2003-06-08 17:15       ` Andrew Cagney
2003-06-08 22:11         ` Andrew Cagney
2003-06-08 22:51           ` Mark Kettenis
2003-06-09  0:22             ` Andrew Cagney
2003-06-09  9:35               ` Mark Kettenis
2003-06-09 14:38                 ` Andrew Cagney
2003-06-09  9:38           ` Mark Kettenis
2003-06-09 14:20             ` Andrew Cagney
2003-06-09 17:43               ` Mark Kettenis
2003-06-09 10:26           ` Mark Kettenis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200306081643.h58GhiTr048427@elgar.kettenis.dyndns.org \
    --to=kettenis@chello.nl \
    --cc=ac131313@redhat.com \
    --cc=gdb-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox