Re: PATCH: Add x86 xml files

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sun, Feb 7, 2010 at 1:50 PM, Daniel Jacobowitz <dan@codesourcery.com> wrote:
> On Thu, Feb 04, 2010 at 04:00:52PM -0800, H.J. Lu wrote:
>> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
>> +<feature name="org.gnu.gdb.i386.core">
>
> In addition to Mark's comments:
>
> * The new features will need to be documented in the manual. ?I'd
> recommend doing that along with the XML files.

I will do that.

> * I can't remember exactly why there are some previously existing
> features with the same name and two possible contents. ?It seems like
> a better idea to call one of these org.gnu.gdb.x86_64.* (or x86-64 or
> amd64 or whatever).

That is done on purpose so that i386-tdep.c can have

---
  /* Get the x86 target description from INFO.  We support both
     32bit and 64bit if BFD64 is defined.  */
  tdesc = info.target_desc;
  if (! tdesc_has_registers (tdesc))
    {
      if (info.bfd_arch_info->bits_per_word == 32)
        tdesc = tdesc_i386;
      else
#ifdef BFD64
        tdesc = tdesc_x86_64;
#else
        return NULL;
#endif
    }

  gdb_assert (tdesc_has_registers (tdesc));

  /* Get core registers.  */
  feature_core = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.core");
  if (feature_core == NULL)
    return NULL;
---

to support both 32bit and 64bit, avoiding unnecessary complication and
code duplication. That is how it is done for PPC/RS6000.

> Otherwise, it seems fine to me.
>

Thanks.

-- 
H.J.