Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Ulrich Weigand" <uweigand@de.ibm.com>
To: cseo@linux.vnet.ibm.com (Carlos Eduardo Seo)
Cc: gdb-patches@sourceware.org (GDB Patches Mailing List)
Subject: Re: [RFC] Add support for PPC Altivec registers in gcore
Date: Mon, 29 Oct 2007 19:24:00 -0000	[thread overview]
Message-ID: <200710291916.l9TJGjHh028749@d12av02.megacenter.de.ibm.com> (raw)
In-Reply-To: <47223F51.4010309@linux.vnet.ibm.com> from "Carlos Eduardo Seo" at Oct 26, 2007 05:26:09 PM

Carlos Eduardo Seo wrote:

> The attached patch makes gcore dump the contents of PPC Altivec registers.
> 
> However, I'm not happy with declaring a bunch of PPC-specific stuff in
> gregset.h. But since FPXREGSET was there, I put everything in that file
> as well.
> 
> I'm open to comments about how to make this better, so please give me
> some thoughts.

> +  if (core_regset_p
> +      && (regset = gdbarch_regset_from_core_section (gdbarch, ".reg-ppc-vmx",
> +						     sizeof (vrregs))) != NULL
> +      && regset->collect_regset != NULL)
> +    regset->collect_regset (regset, regcache, -1,
> +			    &vrregs, sizeof (vrregs));
> +  else
> +    fill_vrregset (regcache, &vrregs);

First of all, you certainly do not need the global fill_vrregset fallback.
In fact, the existing uses of the global "fill_..." functions should be
removed and targets switched over to use the collect_regset method.

For *new* support like this, you should simply require the collect_regset
method to be used, and not provide any fallback.

Also, the current patch as it stands will cause build failures on any
native Linux target but PowerPC, as that function is not defined.


> Index: src/gdb/gregset.h

> +/* For PPC Altivec support  */
> +
> +#define SIZEOF_VRREGS 33*16+4
> +
> +typedef char gdb_vrregset_t[SIZEOF_VRREGS];

Now, the only reason this currently needs to be in common code is
that common code allocates a local variable on the stack, and needs
to know the size required for it.  Everything else is actually not
necessary in common code.

So maybe be should add a new gdbarch variable that points to a 
list of supported register notes, each specified via name and size:

  { { ".reg", sizeof gdb_gregset_t },
    { ".reg2", sizeof gdb_fpregset_t },
    { ".reg-ppx-vmx", sizeof gdb_vrregset_t },
    { NULL } }

Common linux-nat code would walk through the list, instead of 
having to handle each note separately.  (Note that corelow.c
could also walk that list instead of hard-coding note names.)

Probably at least for now the "main" .reg / .reg2 will have to
remain special-cased due to the fill_ fallbacks.  But the 
"special" note types, .reg-xfp for Intel and .reg-ppc-vmx for
PPC, could certainly be switched to the new method.


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


  reply	other threads:[~2007-10-29 19:16 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-26 22:14 Carlos Eduardo Seo
2007-10-29 19:24 ` Ulrich Weigand [this message]
2007-10-30 21:02   ` Carlos Eduardo Seo
2007-10-30 21:18     ` Ulrich Weigand
2007-10-30 21:30       ` Carlos Eduardo Seo
2007-10-30 21:31         ` Ulrich Weigand
2007-10-31 21:14           ` Carlos Eduardo Seo
2007-10-31 21:43             ` Ulrich Weigand
2008-02-08 21:42               ` Carlos Eduardo Seo
2008-02-18 18:42                 ` Ulrich Weigand
2008-02-27 17:07                   ` Carlos Eduardo Seo
2008-03-05 18:27                     ` Ulrich Weigand
2008-03-10 14:22                   ` Carlos Eduardo Seo
2008-03-17 19:07                     ` Ulrich Weigand
2008-03-20 15:31                       ` Carlos Eduardo Seo
2008-03-25 20:13                         ` Ulrich Weigand
2008-03-25 21:31                           ` Andreas Schwab
2008-03-25 21:54                             ` Ulrich Weigand
2008-03-25 22:46                               ` Carlos Eduardo Seo
2008-03-26 11:28                                 ` Ulrich Weigand
2008-03-27  1:52                       ` Carlos Eduardo Seo
2008-03-27  9:00                         ` Andreas Schwab
2008-03-27 19:54                         ` Ulrich Weigand
2008-03-28 20:41                           ` Carlos Eduardo Seo
2008-03-31 19:19                             ` Ulrich Weigand
2008-05-09 19:27                               ` Carlos Eduardo Seo
2008-05-09 20:30                                 ` Ulrich Weigand
2008-05-10  1:33                                   ` Carlos Eduardo Seo
2008-05-14  4:22                                     ` Ulrich Weigand
2008-05-20 18:41                                       ` Carlos Eduardo Seo
2008-05-21 18:46                                         ` Ulrich Weigand
2008-05-22 14:34                                           ` Carlos Eduardo Seo
2008-05-22 18:45                                             ` Ulrich Weigand
2008-05-26 16:26                                               ` Carlos Eduardo Seo
     [not found] <OF67129E0D.852FADB2-ON4125738B.0050E74D-4125738B.005102FF@de.ibm.com>
2007-11-25  4:51 ` Carlos Eduardo Seo
2007-11-26 16:09   ` Ulrich Weigand
2007-11-26 16:12     ` Carlos Eduardo Seo

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=200710291916.l9TJGjHh028749@d12av02.megacenter.de.ibm.com \
    --to=uweigand@de.ibm.com \
    --cc=cseo@linux.vnet.ibm.com \
    --cc=gdb-patches@sourceware.org \
    /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