Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Luis <luis.machado.foss@gmail.com>
To: Ezra.Sitorus@arm.com, gdb-patches@sourceware.org
Cc: thiago.bauermann@linaro.org
Subject: Re: [PATCH v3 1/6] gdb/aarch64: Enable FPMR for AArch64 in gdb on Linux
Date: Mon, 27 Oct 2025 22:32:45 +0000	[thread overview]
Message-ID: <8cd22d6d-d546-4c53-acda-fb8fbdf33eae@gmail.com> (raw)
In-Reply-To: <20251021150300.59860-2-Ezra.Sitorus@arm.com>

Just a minor nit.

On 21/10/2025 16:02, Ezra.Sitorus@arm.com wrote:
> From: Ezra Sitorus <ezra.sitorus@arm.com>
> 
> The Floating Point Mode Register controls the behaviours of FP8
> instructions. This patch add FPMR to GDB if it is enabled on the
> target.
> ---
> Changes from v1->v2:
> * Addressed comments/whitespace/formatting issues.
> * gdb/arch/aarch64.h: operator() takes fpmr into account now.
> * Defined HWCAP2_FPMR in gdb/arch/aarch64.h
> 
> Changes from v2-v3:
> * Formatting fixes.
> * Moved initialisation of fpmr variables closer to where they are used.
> 
> Ezra
> 
>   gdb/aarch64-linux-nat.c       | 57 +++++++++++++++++++++++++++++++++++
>   gdb/aarch64-linux-tdep.c      |  1 +
>   gdb/aarch64-tdep.c            | 15 +++++++++
>   gdb/aarch64-tdep.h            |  9 ++++++
>   gdb/arch/aarch64.c            |  4 +++
>   gdb/arch/aarch64.h            | 12 +++++++-
>   gdb/features/Makefile         |  1 +
>   gdb/features/aarch64-fpmr.c   | 44 +++++++++++++++++++++++++++
>   gdb/features/aarch64-fpmr.xml | 57 +++++++++++++++++++++++++++++++++++
>   9 files changed, 199 insertions(+), 1 deletion(-)
>   create mode 100644 gdb/features/aarch64-fpmr.c
>   create mode 100644 gdb/features/aarch64-fpmr.xml
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 89ecedda57d..503a41c973d 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -604,6 +604,48 @@ store_gcsregs_to_thread (regcache *regcache)
>       perror_with_name (_("Unable to store GCS registers"));
>   }
>   
> +/* Fill GDB's REGCACHE with the FPMR register set content from the
> +   thread associated with REGCACHE.  */
> +
> +static void
> +fetch_fpmr_from_thread (struct regcache *regcache)
> +{
> +  aarch64_gdbarch_tdep *tdep
> +    = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> +
> +    int tid = regcache->ptid ().lwp ();
> +
> +    struct iovec iov;
> +    uint64_t val;
> +    iov.iov_base = &val;
> +    iov.iov_len = sizeof (val);
> +
> +    if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_FPMR, &iov) < 0)
> +      perror_with_name (_("Unable to fetch FPMR register set"));
> +    regcache->raw_supply (tdep->fpmr_regnum, &val);
> +}
> +
> +/* Store the NT_ARM_FPMR register set contents from GDB's REGCACHE to the
> +    thread associated with REGCACHE.  */
> +
> +static void
> +store_fpmr_to_thread (struct regcache *regcache)
> +{
> +  aarch64_gdbarch_tdep *tdep
> +    = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> +
> +  int tid = regcache->ptid ().lwp ();
> +
> +  struct iovec iov;
> +  uint64_t val;
> +  iov.iov_base = &val;
> +  iov.iov_len = sizeof (val);
> +
> +  regcache->raw_collect (tdep->fpmr_regnum, (char *) &val);
> +  if (ptrace (PTRACE_SETREGSET, tid, NT_ARM_FPMR, &iov) < 0)
> +    perror_with_name (_("Unable to store FPMR register set"));
> +}
> +
>   /* The AArch64 version of the "fetch_registers" target_ops method.  Fetch
>      REGNO from the target and place the result into REGCACHE.  */
>   
> @@ -642,6 +684,9 @@ aarch64_fetch_registers (struct regcache *regcache, int regno)
>   
>         if (tdep->has_gcs_linux ())
>   	fetch_gcsregs_from_thread (regcache);
> +
> +      if (tdep->has_fpmr ())
> +	fetch_fpmr_from_thread (regcache);
>       }
>     /* General purpose register?  */
>     else if (regno < AARCH64_V0_REGNUM)
> @@ -679,6 +724,9 @@ aarch64_fetch_registers (struct regcache *regcache, int regno)
>   	   && (regno == tdep->gcs_reg_base || regno == tdep->gcs_linux_reg_base
>   	       || regno == tdep->gcs_linux_reg_base + 1))
>       fetch_gcsregs_from_thread (regcache);
> +  /* FPMR?  */
> +  else if (tdep->has_fpmr () && (regno == tdep->fpmr_regnum))
> +    fetch_fpmr_from_thread (regcache);
>   }
>   
>   /* A version of the "fetch_registers" target_ops method used when running
> @@ -753,6 +801,9 @@ aarch64_store_registers (struct regcache *regcache, int regno)
>   
>         if (tdep->has_gcs_linux ())
>   	store_gcsregs_to_thread (regcache);
> +
> +      if (tdep->has_fpmr ())
> +	store_fpmr_to_thread (regcache);
>       }
>     /* General purpose register?  */
>     else if (regno < AARCH64_V0_REGNUM)
> @@ -784,6 +835,9 @@ aarch64_store_registers (struct regcache *regcache, int regno)
>   	   && (regno == tdep->gcs_reg_base || regno == tdep->gcs_linux_reg_base
>   	       || regno == tdep->gcs_linux_reg_base + 1))
>       store_gcsregs_to_thread (regcache);
> +  /* FPMR?  */
> +  else if (tdep->has_fpmr () && regno == tdep->fpmr_regnum)
> +    store_fpmr_to_thread (regcache);
>   
>     /* PAuth registers are read-only.  */
>   }
> @@ -969,6 +1023,9 @@ aarch64_linux_nat_target::read_description ()
>     if ((hwcap2 & HWCAP2_SME2) || (hwcap2 & HWCAP2_SME2P1))
>       features.sme2 = supports_zt_registers (tid);
>   
> +  /* Check for FPMR.  */
> +  features.fpmr = hwcap2 & HWCAP2_FPMR;
> +
>     return aarch64_read_description (features);
>   }
>   
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 048be4f3532..10b44d978af 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1712,6 +1712,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
>     features.pauth = hwcap & AARCH64_HWCAP_PACA;
>     features.gcs = features.gcs_linux = hwcap & HWCAP_GCS;
>     features.mte = hwcap2 & HWCAP2_MTE;
> +  features.fpmr = hwcap2 & HWCAP2_FPMR;
>   
>     /* Handle the TLS section.  */
>     asection *tls = bfd_get_section_by_name (abfd, ".reg-aarch-tls");
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 500ac77d75a..969554ed571 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -4140,6 +4140,10 @@ aarch64_features_from_target_desc (const struct target_desc *tdesc)
>     features.gcs_linux = (tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.gcs.linux")
>   			!= nullptr);
>   
> +  /* Check for FPMR feature.  */
> +  features.fpmr = (tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpmr")
> +		   != nullptr);
> +
>     return features;
>   }
>   
> @@ -4550,6 +4554,16 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>         num_pseudo_regs += 32;	/* add the Bn scalar register pseudos */
>       }
>   
> +  int fpmr_regnum = -1;
> +  const struct tdesc_feature *feature_fpmr
> +      = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpmr");
> +  if (feature_fpmr != nullptr)
> +    {
> +      fpmr_regnum = num_regs++;
> +      valid_p &= tdesc_numbered_register (feature_fpmr, tdesc_data.get (),
> +					  fpmr_regnum, "fpmr");
> +    }
> +
>     int first_sme_regnum = -1;
>     int first_sme2_regnum = -1;
>     int first_sme_pseudo_regnum = -1;
> @@ -4749,6 +4763,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>     tdep->tls_register_count = tls_register_count;
>     tdep->gcs_reg_base = first_gcs_regnum;
>     tdep->gcs_linux_reg_base = first_gcs_linux_regnum;
> +  tdep->fpmr_regnum = fpmr_regnum;
>   
>     /* Set the SME register set details.  The pseudo-registers will be adjusted
>        later.  */
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index 99e7d26ce4a..9acd29b2d88 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -207,6 +207,15 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base
>     {
>       return gcs_linux_reg_base != -1;
>     }
> +
> +  /* First FPMR register.  This is -1 if FPMR is not supported.  */
> +  int fpmr_regnum = -1;
> +
> +  bool
> +  has_fpmr () const
> +  {
> +    return fpmr_regnum != -1;
> +  }
>   };
>   
>   const target_desc *aarch64_read_description (const aarch64_features &features);
> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
> index dff2bc16003..622138f43b5 100644
> --- a/gdb/arch/aarch64.c
> +++ b/gdb/arch/aarch64.c
> @@ -20,6 +20,7 @@
>   
>   #include "../features/aarch64-core.c"
>   #include "../features/aarch64-fpu.c"
> +#include "../features/aarch64-fpmr.c"
>   #include "../features/aarch64-sve.c"
>   #include "../features/aarch64-pauth.c"
>   #include "../features/aarch64-mte.c"
> @@ -73,6 +74,9 @@ aarch64_create_target_description (const aarch64_features &features)
>     if (features.gcs_linux)
>       regnum = create_feature_aarch64_gcs_linux (tdesc.get (), regnum);
>   
> +  if (features.fpmr)
> +    regnum = create_feature_aarch64_fpmr (tdesc.get (), regnum);
> +
>     return tdesc.release ();
>   }
>   
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index 679d845df74..8f4ba9c9e0c 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -34,6 +34,7 @@ struct aarch64_features
>     uint64_t vq = 0;
>     bool pauth = false;
>     bool mte = false;
> +  bool fpmr = false;
>   
>     /* A positive TLS value indicates the number of TLS registers available.  */
>     uint8_t tls = 0;
> @@ -68,7 +69,8 @@ inline bool operator==(const aarch64_features &lhs, const aarch64_features &rhs)
>       && lhs.svq == rhs.svq
>       && lhs.sme2 == rhs.sme2
>       && lhs.gcs == rhs.gcs
> -    && lhs.gcs_linux == rhs.gcs_linux;
> +    && lhs.gcs_linux == rhs.gcs_linux
> +    && lhs.fpmr == rhs.fpmr;
>   }
>   
>   namespace std
> @@ -94,6 +96,9 @@ namespace std
>   
>         /* SME2 feature.  */
>         h = h << 1 | features.sme2;
> +
> +      /* FPMR feature.  */
> +      h = h << 1 | features.fpmr;
>         return h;
>       }
>     };
> @@ -238,4 +243,9 @@ enum aarch64_regnum
>   /* Size of the SME2 ZT0 register in bytes.  */
>   #define AARCH64_SME2_ZT0_SIZE 64
>   
> +/* Feature check for Floating Point Mode Register.  */
> +#ifndef HWCAP2_FPMR
> +#define HWCAP2_FPMR (1ULL << 48)
> +#endif /* HWCAP2_FPMR */
> +
>   #endif /* GDB_ARCH_AARCH64_H */
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index d17c349b6cf..ed1b8bf119c 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -201,6 +201,7 @@ $(outdir)/%.dat: %.xml number-regs.xsl sort-regs.xsl gdbserver-regs.xsl
>   # For targets with feature based target descriptions,
>   # the set of xml files we'll generate .c files for GDB from.
>   FEATURE_XMLFILES = aarch64-core.xml \
> +	aarch64-fpmr.xml \
>   	aarch64-fpu.xml \
>   	aarch64-pauth.xml \
>   	aarch64-mte.xml \
> diff --git a/gdb/features/aarch64-fpmr.c b/gdb/features/aarch64-fpmr.c
> new file mode 100644
> index 00000000000..a372b12530b
> --- /dev/null
> +++ b/gdb/features/aarch64-fpmr.c
> @@ -0,0 +1,44 @@
> +/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
> +  Original: aarch64-fpmr.xml */
> +
> +#include "gdbsupport/tdesc.h"
> +
> +static int
> +create_feature_aarch64_fpmr (struct target_desc *result, long regnum)
> +{
> +  struct tdesc_feature *feature;
> +
> +  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpmr");
> +  tdesc_type_with_fields *type_with_fields;
> +  type_with_fields = tdesc_create_enum (feature, "fp8_fmt", 3);
> +  tdesc_add_enum_value (type_with_fields, 0, "E5M2");
> +  tdesc_add_enum_value (type_with_fields, 1, "E4M3");
> +
> +  type_with_fields = tdesc_create_enum (feature, "osc", 1);
> +  tdesc_add_enum_value (type_with_fields, 0, "Inf/NaN");
> +  tdesc_add_enum_value (type_with_fields, 1, "MaxNormal");
> +
> +  type_with_fields = tdesc_create_enum (feature, "osm", 1);
> +  tdesc_add_enum_value (type_with_fields, 0, "Inf");
> +  tdesc_add_enum_value (type_with_fields, 1, "MaxNormal");
> +
> +  type_with_fields = tdesc_create_flags (feature, "fpmr_flags", 8);
> +  tdesc_type *field_type;
> +  field_type = tdesc_named_type (feature, "fp8_fmt");
> +  tdesc_add_typed_bitfield (type_with_fields, "F8S1", 0, 2, field_type);
> +  field_type = tdesc_named_type (feature, "fp8_fmt");
> +  tdesc_add_typed_bitfield (type_with_fields, "F8S2", 3, 5, field_type);
> +  field_type = tdesc_named_type (feature, "fp8_fmt");
> +  tdesc_add_typed_bitfield (type_with_fields, "F8D", 6, 8, field_type);
> +  field_type = tdesc_named_type (feature, "osm");
> +  tdesc_add_typed_bitfield (type_with_fields, "OSM", 14, 14, field_type);
> +  field_type = tdesc_named_type (feature, "osc");
> +  tdesc_add_typed_bitfield (type_with_fields, "OSC", 15, 15, field_type);
> +  tdesc_add_bitfield (type_with_fields, "LSCALE", 16, 22);
> +  field_type = tdesc_named_type (feature, "int8");
> +  tdesc_add_typed_bitfield (type_with_fields, "NSCALE", 24, 31, field_type);
> +  tdesc_add_bitfield (type_with_fields, "LSCALE2", 32, 37);
> +
> +  tdesc_create_reg (feature, "fpmr", regnum++, 1, NULL, 64, "fpmr_flags");
> +  return regnum;
> +}
> diff --git a/gdb/features/aarch64-fpmr.xml b/gdb/features/aarch64-fpmr.xml
> new file mode 100644
> index 00000000000..edbae6d107c
> --- /dev/null
> +++ b/gdb/features/aarch64-fpmr.xml
> @@ -0,0 +1,57 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2025 Free Software Foundation, Inc.
> +
> +     Copying and distribution of this file, with or without modification,
> +     are permitted in any medium without royalty provided the copyright
> +     notice and this notice are preserved.  -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.aarch64.fpmr">
> +
> +  <!-- FP8 format for F8S1, F8S2 and F8D fields. This is either E5M2 or

Formatting: Two spaces after period (since you've done it elsewhere in 
the xml file. It is best to make it consistent.

> +       E4M3.  -->
> +  <enum id="fp8_fmt" size="3">
> +    <evalue name="E5M2" value="0"/>
> +    <evalue name="E4M3" value="1"/>
> +  </enum>
> +
> +  <!-- Overflow saturation for FP8 convert instructions.  Specifies the result
> +       when a floating-point overflow exception is detected.  -->
> +  <enum id="osc" size="1">
> +    <!-- Infinity or NaN is generated.  -->
> +    <evalue name="Inf/NaN" value="0"/>
> +    <!-- Maximum normal number is generated.  -->
> +    <evalue name="MaxNormal" value="1"/>
> +  </enum>
> +
> +  <!-- Overflow saturation for FP8 multiplication instructions.  Specifies the
> +       result when a floating-point overflow exception is detected.  -->
> +    <enum id="osm" size="1">
> +    <!-- Infinity generated.  -->
> +    <evalue name="Inf" value="0"/>
> +    <!-- Maximum normal number is generated.  -->
> +    <evalue name="MaxNormal" value="1"/>
> +  </enum>
> +
> +  <flags id="fpmr_flags" size="8">
> +    <!-- SRC1 Format.  -->
> +    <field name="F8S1" start="0" end="2" type="fp8_fmt"/>
> +    <!-- SRC2 Format.  -->
> +    <field name="F8S2" start="3" end="5" type="fp8_fmt"/>
> +    <!-- F8D Format.  -->
> +    <field name="F8D" start="6" end="8" type="fp8_fmt"/>
> +    <!-- OSM.  -->
> +    <field name="OSM" start="14" end="14" type="osm"/>
> +    <!-- OSC.  -->
> +    <field name="OSC" start="15" end="15" type="osc"/>
> +    <!-- LSCALE.  -->
> +    <field name="LSCALE" start="16" end="22"/>
> +    <!-- NSCALE.  -->
> +    <field name="NSCALE" start="24" end="31" type="int8"/>
> +    <!-- LSCALE2.  -->
> +    <field name="LSCALE2" start="32" end="37"/>
> +  </flags>
> +
> +  <reg name="fpmr" bitsize="64" type="fpmr_flags"/>
> +
> +</feature>

Otherwise this is OK with that nit fixed. Thanks!

Approved-By: Luis Machado <luis.machado.foss@gmail.com>

I think we only need to address the minor issues with patch 5 and 6.

  reply	other threads:[~2025-10-27 22:34 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-21 15:02 [PATCH v3 0/6] gdb/aarch64: Support for FPMR Ezra.Sitorus
2025-10-21 15:02 ` [PATCH v3 1/6] gdb/aarch64: Enable FPMR for AArch64 in gdb on Linux Ezra.Sitorus
2025-10-27 22:32   ` Luis [this message]
2025-10-21 15:02 ` [PATCH v3 2/6] gdbserver/aarch64: Enable FPMR for AArch64 in gdbserver " Ezra.Sitorus
2025-10-27 22:26   ` Luis
2025-10-21 15:02 ` [PATCH v3 3/6] gdb/aarch64: signal frame support for fpmr Ezra.Sitorus
2025-10-27 22:25   ` Luis
2025-10-21 15:02 ` [PATCH v3 4/6] gdb/aarch64: core file support for FPMR Ezra.Sitorus
2025-10-27 22:23   ` Luis
2025-10-21 15:02 ` [PATCH v3 5/6] gdb/aarch64: Tests " Ezra.Sitorus
2025-10-27 22:19   ` Luis
2025-10-21 15:03 ` [PATCH v3 6/6] gdb/doc: Document AArch64 FPMR support Ezra.Sitorus
2025-10-21 15:35   ` Eli Zaretskii
2025-10-22  9:25     ` Ezra Sitorus
2025-10-22 11:53       ` Eli Zaretskii
2025-10-27 22:08   ` Luis

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=8cd22d6d-d546-4c53-acda-fb8fbdf33eae@gmail.com \
    --to=luis.machado.foss@gmail.com \
    --cc=Ezra.Sitorus@arm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=thiago.bauermann@linaro.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