* [GDB][PATCH] arm: Add support for mve vector predicate status and control register (vpr) and pseudo register p0.
@ 2021-07-02 8:47 Srinath Parvathaneni
2021-07-02 15:44 ` Alan Hayward
2021-07-05 10:39 ` Luis Machado
0 siblings, 2 replies; 4+ messages in thread
From: Srinath Parvathaneni @ 2021-07-02 8:47 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 18903 bytes --]
Hello,
This patch add support for mve vector predication status and control register (vpr) and pseudo register p0.
Pseudo register p0 is the least significant bits of vpr and can be accessed as $p0, $vpr.p0.
For more information about the register please refer to [1].
[1] https://developer.arm.com/documentation/ddi0553/bn
Built gdb for arm-none-eabi target and regtested.
Ok for master?
Regards,
Srinath
gdb/ChangeLog:
2021-07-02 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
* arch/arm.c (arm_create_mprofile_target_description): Add a new case
for m_type ARM_M_TYPE_MVE.
(ARM_M_TYPE_MVE): Add m_type case to create feature profiles.
* arch/arm.h (ARM_VPR_REGNUM): Add to enum gdb_regnum.
(ARM_M_TYPE_MVE): Add to enum arm_m_profile_type.
* arm-tdep.c (arm_register_type): Modify function to return
builtin_int16 on receiving mve pseudo register p0.
(arm_register_name): Rename to arm_pseudo_register_name.
(arm_pseudo_register_name): Renamed arm_register_name and add a case
for mve pseudo reigster p0.
(arm_mve_pseudo_read): Define function to read value from VPR.P0.
(arm_pseudo_read): Add case for mve pseudo reigster p0.
(arm_mve_pseudo_write): Define function to write value to VPR.P0.
(arm_pseudo_write): Add case for mve pseudo reigster p0.
(arm_register_g_packet_guesses): Allocate memory for registers of
ARM_M_TYPE_MVE m_type.
(arm_gdbarch_init): Add support for new feature arm.m-profile-with-mve
and new reigster ARM_VPR_REGNUM.
(arm_dump_tdep): Print arm_dump_tdep when mve pseudo is present.
* arm-tdep.h (struct gdbarch_tdep): Define have_mve_pseudos.
* features/Makefile (arm/arm-m-profile-with-mve.xml): Add m-profile
mve feature XML file.
* features/arm/arm-m-profile-with-mve.c: Renegerated file.
* features/arm/arm-m-profile-with-mve.xml: New file.
gdb/doc/ChangeLog:
2021-07-02 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
* gdb.texinfo (org.gnu.gdb.arm.m-profile-mve): Add mve feature entry.
############### Attachment also inlined for ease of reply ###############
diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
index b0eb2ae445f2c716dd21dbbb62b87ed678d93722..8e41fa73d77dc1caa4749ae9d82cbe6bc615ec9c 100644
--- a/gdb/arch/arm.h
+++ b/gdb/arch/arm.h
@@ -49,6 +49,7 @@ enum gdb_regnum {
ARM_D0_REGNUM, /* VFP double-precision registers. */
ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
ARM_FPSCR_REGNUM,
+ ARM_VPR_REGNUM,
ARM_NUM_REGS,
@@ -83,6 +84,7 @@ enum arm_m_profile_type {
ARM_M_TYPE_M_PROFILE,
ARM_M_TYPE_VFP_D16,
ARM_M_TYPE_WITH_FPA,
+ ARM_M_TYPE_MVE,
ARM_M_TYPE_INVALID
};
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
index faa2b4fbd4b21588d46c5515b96a838d9ad94553..25065f2a57e774c38bf2384bf29aa62ef60eed04 100644
--- a/gdb/arch/arm.c
+++ b/gdb/arch/arm.c
@@ -27,6 +27,7 @@
#include "../features/arm/xscale-iwmmxt.c"
#include "../features/arm/arm-m-profile.c"
#include "../features/arm/arm-m-profile-with-fpa.c"
+#include "../features/arm/arm-m-profile-with-mve.c"
/* See arm.h. */
@@ -439,6 +440,12 @@ arm_create_mprofile_target_description (arm_m_profile_type m_type)
regnum = create_feature_arm_arm_m_profile_with_fpa (tdesc, regnum);
break;
+ case ARM_M_TYPE_MVE:
+ regnum = create_feature_arm_arm_m_profile (tdesc, regnum);
+ regnum = create_feature_arm_arm_vfpv2 (tdesc, regnum);
+ regnum = create_feature_arm_arm_m_profile_with_mve (tdesc, regnum);
+ break;
+
default:
error (_("Invalid Arm M type: %d"), m_type);
}
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 629c3b8ac685f411fe7eba45eb37ded3ea9d2a82..22b1a6ea0a02984a122ddc19f300d46f05c056dc 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -106,6 +106,10 @@ struct gdbarch_tdep
NEON registers? Requires
have_vfp_pseudos. */
bool have_neon; /* Do we have a NEON unit? */
+ bool have_mve_pseudos; /* We are synthesizing the p0 register from VPR
+ register. For this we need have_vfp_pseudos
+ and have_neon_pseudos true, so that p0 is
+ added to the pseudo register list. */
bool is_m; /* Does the target follow the "M" profile. */
CORE_ADDR lowest_pc; /* Lowest address at which instructions
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index a214f22d7ad10740c67509c4018b4494c5e7f7fe..7f8f7bc2ca3852f23b512a7ab1ef1aa8a1d4b49e 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -4089,10 +4089,15 @@ arm_register_type (struct gdbarch *gdbarch, int regnum)
&& regnum >= num_regs && regnum < num_regs + 32)
return builtin_type (gdbarch)->builtin_float;
- if (gdbarch_tdep (gdbarch)->have_neon_pseudos
- && regnum >= num_regs + 32 && regnum < num_regs + 32 + 16)
+ if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= num_regs + 32
+ && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < num_regs + 48)
+ || (gdbarch_tdep (gdbarch)->have_mve_pseudos
+ && regnum < num_regs + 40)))
return arm_neon_quad_type (gdbarch);
+ if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == num_regs + 40)
+ return builtin_type (gdbarch)->builtin_int16;
+
/* If the target description has register information, we are only
in this function so that we can override the types of
double-precision registers for NEON. */
@@ -8581,7 +8586,7 @@ show_disassembly_style_sfunc (struct ui_file *file, int from_tty,
\f
/* Return the ARM register name corresponding to register I. */
static const char *
-arm_register_name (struct gdbarch *gdbarch, int i)
+arm_pseudo_register_name (struct gdbarch *gdbarch, int i)
{
const int num_regs = gdbarch_num_regs (gdbarch);
@@ -8598,8 +8603,9 @@ arm_register_name (struct gdbarch *gdbarch, int i)
return vfp_pseudo_names[i - num_regs];
}
- if (gdbarch_tdep (gdbarch)->have_neon_pseudos
- && i >= num_regs + 32 && i < num_regs + 32 + 16)
+ if (gdbarch_tdep (gdbarch)->have_neon_pseudos && i >= num_regs + 32
+ && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && i < num_regs + 48)
+ || (gdbarch_tdep (gdbarch)->have_mve_pseudos && i < num_regs + 40)))
{
static const char *const neon_pseudo_names[] = {
"q0", "q1", "q2", "q3", "q4", "q5", "q6", "q7",
@@ -8608,6 +8614,12 @@ arm_register_name (struct gdbarch *gdbarch, int i)
return neon_pseudo_names[i - num_regs - 32];
}
+ if (gdbarch_tdep (gdbarch)->have_mve_pseudos && i == num_regs + 40)
+ {
+ static const char *const mve_pseudo_names[] = {"p0"};
+
+ return mve_pseudo_names[i - num_regs - 40];
+ }
if (i >= ARRAY_SIZE (arm_register_names))
/* These registers are only supported on targets which supply
@@ -8741,6 +8753,35 @@ arm_neon_quad_read (struct gdbarch *gdbarch, readable_regcache *regcache,
return REG_VALID;
}
+/* Store pseudo register P0 to BUF, P0 is least significant 16 bits of
+ Armv8.1-M VPR register. */
+static enum register_status
+arm_mve_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+ int regnum, gdb_byte *buf)
+{
+ char name_buf[4];
+ gdb_byte reg_buf[4];
+ int vpr_regnum;
+ enum register_status status;
+
+ if (regnum == 0)
+ xsnprintf (name_buf, sizeof (name_buf), "vpr");
+ else
+ gdb_assert_not_reached ("wrong pseudo regnum for MVE.");
+
+ vpr_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
+ strlen (name_buf));
+ /* Read the actual value in the VPR register. */
+ status = regcache->raw_read (vpr_regnum, reg_buf);
+ if (status != REG_VALID)
+ gdb_assert_not_reached ("No vpr register for MVE.");
+
+ /* Copy only the last 2 bytes of reg_buf to buf. */
+ memcpy (buf, reg_buf + 2, 2);
+
+ return REG_VALID;
+}
+
static enum register_status
arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
int regnum, gdb_byte *buf)
@@ -8753,9 +8794,13 @@ arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
gdb_assert (regnum >= num_regs);
regnum -= num_regs;
- if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
+ if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32
+ && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 48)
+ || (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 40)))
/* Quad-precision register. */
return arm_neon_quad_read (gdbarch, regcache, regnum - 32, buf);
+ else if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == 40)
+ return arm_mve_pseudo_read (gdbarch, regcache, regnum - 40, buf);
else
{
enum register_status status;
@@ -8809,6 +8854,35 @@ arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache,
regcache->raw_write (double_regnum + 1, buf + offset);
}
+/* BUF needs to be stored to pseudo register P0, which is
+ least significant 16 bits of Armv8.1-M VPR register. */
+static void
+arm_mve_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
+ int regnum, const gdb_byte *buf)
+{
+ char name_buf[4];
+ gdb_byte reg_buf[4];
+ int vpr_regnum;
+ enum register_status status;
+
+ if (regnum == 0)
+ xsnprintf (name_buf, sizeof (name_buf), "vpr");
+ else
+ gdb_assert_not_reached ("wrong pseudo regnum for MVE.");
+
+ vpr_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
+ strlen (name_buf));
+ /* Read the actual value in the VPR register. */
+ status = regcache->raw_read (vpr_regnum, reg_buf);
+ if (status != REG_VALID)
+ gdb_assert_not_reached ("No vpr register for MVE.");
+
+ /* Update the last 2 bytes of reg_buf with the buf value. */
+ memcpy (reg_buf+2, buf, 2);
+
+ regcache->raw_write (vpr_regnum, reg_buf);
+}
+
static void
arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
int regnum, const gdb_byte *buf)
@@ -8821,9 +8895,13 @@ arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
gdb_assert (regnum >= num_regs);
regnum -= num_regs;
- if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
+ if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32
+ && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 48)
+ || (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 40)))
/* Quad-precision register. */
arm_neon_quad_write (gdbarch, regcache, regnum - 32, buf);
+ else if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == 40)
+ arm_mve_pseudo_write (gdbarch, regcache, regnum - 40, buf);
else
{
/* Single-precision register. */
@@ -8921,6 +8999,11 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
register_remote_g_packet_guess (gdbarch,
ARM_CORE_REGS_SIZE + ARM_VFP2_REGS_SIZE,
tdesc);
+ /* M-profile plus MVE. */
+ tdesc = arm_read_mprofile_description (ARM_M_TYPE_MVE);
+ register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE
+ + ARM_VFP2_REGS_SIZE
+ + ARM_INT_REGISTER_SIZE, tdesc);
}
/* Otherwise we don't have a useful guess. */
@@ -8971,8 +9054,10 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdesc_arch_data_up tdesc_data;
int i;
bool is_m = false;
+ bool is_mve = false;
int vfp_register_count = 0;
bool have_vfp_pseudos = false, have_neon_pseudos = false;
+ bool have_mve_pseudos = false;
bool have_wmmx_registers = false;
bool have_neon = false;
bool have_fpa_registers = true;
@@ -9091,6 +9176,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (!tdesc_has_registers (tdesc)
&& (attr_arch == TAG_CPU_ARCH_V6_M
|| attr_arch == TAG_CPU_ARCH_V6S_M
+ || attr_arch == TAG_CPU_ARCH_V8_1M_MAIN
|| attr_profile == 'M'))
is_m = true;
#endif
@@ -9169,8 +9255,20 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
ARM_PC_REGNUM,
arm_pc_names);
if (is_m)
- valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
- ARM_PS_REGNUM, "xpsr");
+ {
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ ARM_PS_REGNUM, "xpsr");
+ feature = tdesc_find_feature (tdesc,"org.gnu.gdb.arm.m-profile-mve");
+
+ if (feature != NULL)
+ {
+ is_mve = true;
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ ARM_VPR_REGNUM, "vpr");
+ if (tdesc_unnumbered_register (feature, "p0") == 0)
+ have_mve_pseudos = true;
+ }
+ }
else
valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
ARM_PS_REGNUM, "cpsr");
@@ -9270,7 +9368,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
does not support that. */
feature = tdesc_find_feature (tdesc,
"org.gnu.gdb.arm.neon");
- if (feature != NULL)
+ /* If we have VFP and no NEON, check for MVE. MVE uses NEON pseudos
+ q0 to q7. */
+ if (feature != NULL || is_mve)
{
/* NEON requires 32 double-precision registers. */
if (i != 32)
@@ -9282,7 +9382,8 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (tdesc_unnumbered_register (feature, "q0") == 0)
have_neon_pseudos = true;
- have_neon = true;
+ if (!is_mve)
+ have_neon = true;
}
}
}
@@ -9332,6 +9433,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdep->vfp_register_count = vfp_register_count;
tdep->have_vfp_pseudos = have_vfp_pseudos;
tdep->have_neon_pseudos = have_neon_pseudos;
+ tdep->have_mve_pseudos = have_mve_pseudos;
tdep->have_neon = have_neon;
arm_register_g_packet_guesses (gdbarch);
@@ -9432,7 +9534,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
set_gdbarch_dwarf2_reg_to_regnum (gdbarch, arm_dwarf_reg_to_regnum);
set_gdbarch_register_sim_regno (gdbarch, arm_register_sim_regno);
- set_gdbarch_register_name (gdbarch, arm_register_name);
+ set_gdbarch_register_name (gdbarch, arm_pseudo_register_name);
/* Returning results. */
set_gdbarch_return_value (gdbarch, arm_return_value);
@@ -9509,8 +9611,11 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
little more care in numbering will be needed. */
int num_pseudos = 32;
- if (have_neon_pseudos)
+ if (have_neon_pseudos && !is_mve)
num_pseudos += 16;
+ /* q0-q7 neon pseduo registers + p0 mve pseduo register. */
+ else if (have_mve_pseudos && is_mve)
+ num_pseudos += 9;
set_gdbarch_num_pseudo_regs (gdbarch, num_pseudos);
set_gdbarch_pseudo_register_read (gdbarch, arm_pseudo_read);
set_gdbarch_pseudo_register_write (gdbarch, arm_pseudo_write);
@@ -9518,7 +9623,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (tdesc_data != nullptr)
{
- set_tdesc_pseudo_register_name (gdbarch, arm_register_name);
+ set_tdesc_pseudo_register_name (gdbarch, arm_pseudo_register_name);
tdesc_use_registers (gdbarch, tdesc, std::move (tdesc_data));
@@ -9564,6 +9669,8 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
(int) tdep->have_neon_pseudos);
fprintf_unfiltered (file, _("arm_dump_tdep: have_neon = %i\n"),
(int) tdep->have_neon);
+ fprintf_unfiltered (file, _("arm_dump_tdep: have_mve_pseudos = %i\n"),
+ (int) tdep->have_mve_pseudos);
fprintf_unfiltered (file, _("arm_dump_tdep: Lowest pc = 0x%lx\n"),
(unsigned long) tdep->lowest_pc);
}
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8bff27c940dec2c5a627459b343a33f3b4f886a1..8348565a6a0c2ba16049030e00e270d2c641ddaa 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -45329,6 +45329,9 @@ and @samp{xpsr}.
The @samp{org.gnu.gdb.arm.fpa} feature is optional. If present, it
should contain registers @samp{f0} through @samp{f7} and @samp{fps}.
+The @samp{org.gnu.gdb.arm.m-profile-mve} feature is optional. If present, it
+should contain register @samp{vpr}.
+
The @samp{org.gnu.gdb.xscale.iwmmxt} feature is optional. If present,
it should contain at least registers @samp{wR0} through @samp{wR15} and
@samp{wCGR0} through @samp{wCGR3}. The @samp{wCID}, @samp{wCon},
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 689603847a001ae0297558cd1fde1242b1c9800c..51db08fe9fd2e8197613b971b8f9fbe2b8daf0f3 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -208,6 +208,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
arm/arm-core.xml \
arm/arm-fpa.xml \
arm/arm-m-profile.xml \
+ arm/arm-m-profile-with-mve.xml \
arm/arm-m-profile-with-fpa.xml \
arm/arm-vfpv2.xml \
arm/arm-vfpv3.xml \
diff --git a/gdb/features/arm/arm-m-profile-with-mve.c b/gdb/features/arm/arm-m-profile-with-mve.c
new file mode 100644
index 0000000000000000000000000000000000000000..b23e77f7427b197a927d31cf1bc3a9f5a2354d4e
--- /dev/null
+++ b/gdb/features/arm/arm-m-profile-with-mve.c
@@ -0,0 +1,21 @@
+/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
+ Original: arm-m-profile-with-mve.xml */
+
+#include "gdbsupport/tdesc.h"
+
+static int
+create_feature_arm_arm_m_profile_with_mve (struct target_desc *result, long regnum)
+{
+ struct tdesc_feature *feature;
+
+ feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile-mve");
+ tdesc_type_with_fields *type_with_fields;
+ type_with_fields = tdesc_create_flags (feature, "vpr_reg", 4);
+ tdesc_add_bitfield (type_with_fields, "P0", 0, 15);
+ tdesc_add_bitfield (type_with_fields, "MASK01", 16, 19);
+ tdesc_add_bitfield (type_with_fields, "MASK23", 20, 23);
+ tdesc_add_bitfield (type_with_fields, "RES0", 24, 31);
+
+ tdesc_create_reg (feature, "vpr", regnum++, 1, NULL, 32, "vpr_reg");
+ return regnum;
+}
diff --git a/gdb/features/arm/arm-m-profile-with-mve.xml b/gdb/features/arm/arm-m-profile-with-mve.xml
new file mode 100644
index 0000000000000000000000000000000000000000..35b17351e0ca0c418c138c3eb711b4e54daac95c
--- /dev/null
+++ b/gdb/features/arm/arm-m-profile-with-mve.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2021 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.arm.m-profile-mve">
+ <flags id="vpr_reg" size="4">
+ <!-- ARMv8.1-M and MVE: Unprivileged and privileged Access. -->
+ <field name="P0" start="0" end="15"/>
+ <!-- ARMv8.1-M: Privileged Access only. -->
+ <field name="MASK01" start="16" end="19"/>
+ <!-- ARMv8.1-M: Privileged Access only. -->
+ <field name="MASK23" start="20" end="23"/>
+ <!-- ARMv8.1-M: Privileged Access only. -->
+ <field name="RES0" start="24" end="31"/>
+ </flags>
+ <reg name="vpr" bitsize="32" type="vpr_reg"/>
+</feature>
[-- Attachment #2: rb14189.patch --]
[-- Type: text/plain, Size: 16846 bytes --]
diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
index b0eb2ae445f2c716dd21dbbb62b87ed678d93722..8e41fa73d77dc1caa4749ae9d82cbe6bc615ec9c 100644
--- a/gdb/arch/arm.h
+++ b/gdb/arch/arm.h
@@ -49,6 +49,7 @@ enum gdb_regnum {
ARM_D0_REGNUM, /* VFP double-precision registers. */
ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
ARM_FPSCR_REGNUM,
+ ARM_VPR_REGNUM,
ARM_NUM_REGS,
@@ -83,6 +84,7 @@ enum arm_m_profile_type {
ARM_M_TYPE_M_PROFILE,
ARM_M_TYPE_VFP_D16,
ARM_M_TYPE_WITH_FPA,
+ ARM_M_TYPE_MVE,
ARM_M_TYPE_INVALID
};
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
index faa2b4fbd4b21588d46c5515b96a838d9ad94553..25065f2a57e774c38bf2384bf29aa62ef60eed04 100644
--- a/gdb/arch/arm.c
+++ b/gdb/arch/arm.c
@@ -27,6 +27,7 @@
#include "../features/arm/xscale-iwmmxt.c"
#include "../features/arm/arm-m-profile.c"
#include "../features/arm/arm-m-profile-with-fpa.c"
+#include "../features/arm/arm-m-profile-with-mve.c"
/* See arm.h. */
@@ -439,6 +440,12 @@ arm_create_mprofile_target_description (arm_m_profile_type m_type)
regnum = create_feature_arm_arm_m_profile_with_fpa (tdesc, regnum);
break;
+ case ARM_M_TYPE_MVE:
+ regnum = create_feature_arm_arm_m_profile (tdesc, regnum);
+ regnum = create_feature_arm_arm_vfpv2 (tdesc, regnum);
+ regnum = create_feature_arm_arm_m_profile_with_mve (tdesc, regnum);
+ break;
+
default:
error (_("Invalid Arm M type: %d"), m_type);
}
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 629c3b8ac685f411fe7eba45eb37ded3ea9d2a82..22b1a6ea0a02984a122ddc19f300d46f05c056dc 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -106,6 +106,10 @@ struct gdbarch_tdep
NEON registers? Requires
have_vfp_pseudos. */
bool have_neon; /* Do we have a NEON unit? */
+ bool have_mve_pseudos; /* We are synthesizing the p0 register from VPR
+ register. For this we need have_vfp_pseudos
+ and have_neon_pseudos true, so that p0 is
+ added to the pseudo register list. */
bool is_m; /* Does the target follow the "M" profile. */
CORE_ADDR lowest_pc; /* Lowest address at which instructions
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index a214f22d7ad10740c67509c4018b4494c5e7f7fe..7f8f7bc2ca3852f23b512a7ab1ef1aa8a1d4b49e 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -4089,10 +4089,15 @@ arm_register_type (struct gdbarch *gdbarch, int regnum)
&& regnum >= num_regs && regnum < num_regs + 32)
return builtin_type (gdbarch)->builtin_float;
- if (gdbarch_tdep (gdbarch)->have_neon_pseudos
- && regnum >= num_regs + 32 && regnum < num_regs + 32 + 16)
+ if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= num_regs + 32
+ && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < num_regs + 48)
+ || (gdbarch_tdep (gdbarch)->have_mve_pseudos
+ && regnum < num_regs + 40)))
return arm_neon_quad_type (gdbarch);
+ if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == num_regs + 40)
+ return builtin_type (gdbarch)->builtin_int16;
+
/* If the target description has register information, we are only
in this function so that we can override the types of
double-precision registers for NEON. */
@@ -8581,7 +8586,7 @@ show_disassembly_style_sfunc (struct ui_file *file, int from_tty,
\f
/* Return the ARM register name corresponding to register I. */
static const char *
-arm_register_name (struct gdbarch *gdbarch, int i)
+arm_pseudo_register_name (struct gdbarch *gdbarch, int i)
{
const int num_regs = gdbarch_num_regs (gdbarch);
@@ -8598,8 +8603,9 @@ arm_register_name (struct gdbarch *gdbarch, int i)
return vfp_pseudo_names[i - num_regs];
}
- if (gdbarch_tdep (gdbarch)->have_neon_pseudos
- && i >= num_regs + 32 && i < num_regs + 32 + 16)
+ if (gdbarch_tdep (gdbarch)->have_neon_pseudos && i >= num_regs + 32
+ && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && i < num_regs + 48)
+ || (gdbarch_tdep (gdbarch)->have_mve_pseudos && i < num_regs + 40)))
{
static const char *const neon_pseudo_names[] = {
"q0", "q1", "q2", "q3", "q4", "q5", "q6", "q7",
@@ -8608,6 +8614,12 @@ arm_register_name (struct gdbarch *gdbarch, int i)
return neon_pseudo_names[i - num_regs - 32];
}
+ if (gdbarch_tdep (gdbarch)->have_mve_pseudos && i == num_regs + 40)
+ {
+ static const char *const mve_pseudo_names[] = {"p0"};
+
+ return mve_pseudo_names[i - num_regs - 40];
+ }
if (i >= ARRAY_SIZE (arm_register_names))
/* These registers are only supported on targets which supply
@@ -8741,6 +8753,35 @@ arm_neon_quad_read (struct gdbarch *gdbarch, readable_regcache *regcache,
return REG_VALID;
}
+/* Store pseudo register P0 to BUF, P0 is least significant 16 bits of
+ Armv8.1-M VPR register. */
+static enum register_status
+arm_mve_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+ int regnum, gdb_byte *buf)
+{
+ char name_buf[4];
+ gdb_byte reg_buf[4];
+ int vpr_regnum;
+ enum register_status status;
+
+ if (regnum == 0)
+ xsnprintf (name_buf, sizeof (name_buf), "vpr");
+ else
+ gdb_assert_not_reached ("wrong pseudo regnum for MVE.");
+
+ vpr_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
+ strlen (name_buf));
+ /* Read the actual value in the VPR register. */
+ status = regcache->raw_read (vpr_regnum, reg_buf);
+ if (status != REG_VALID)
+ gdb_assert_not_reached ("No vpr register for MVE.");
+
+ /* Copy only the last 2 bytes of reg_buf to buf. */
+ memcpy (buf, reg_buf + 2, 2);
+
+ return REG_VALID;
+}
+
static enum register_status
arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
int regnum, gdb_byte *buf)
@@ -8753,9 +8794,13 @@ arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
gdb_assert (regnum >= num_regs);
regnum -= num_regs;
- if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
+ if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32
+ && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 48)
+ || (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 40)))
/* Quad-precision register. */
return arm_neon_quad_read (gdbarch, regcache, regnum - 32, buf);
+ else if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == 40)
+ return arm_mve_pseudo_read (gdbarch, regcache, regnum - 40, buf);
else
{
enum register_status status;
@@ -8809,6 +8854,35 @@ arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache,
regcache->raw_write (double_regnum + 1, buf + offset);
}
+/* BUF needs to be stored to pseudo register P0, which is
+ least significant 16 bits of Armv8.1-M VPR register. */
+static void
+arm_mve_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
+ int regnum, const gdb_byte *buf)
+{
+ char name_buf[4];
+ gdb_byte reg_buf[4];
+ int vpr_regnum;
+ enum register_status status;
+
+ if (regnum == 0)
+ xsnprintf (name_buf, sizeof (name_buf), "vpr");
+ else
+ gdb_assert_not_reached ("wrong pseudo regnum for MVE.");
+
+ vpr_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
+ strlen (name_buf));
+ /* Read the actual value in the VPR register. */
+ status = regcache->raw_read (vpr_regnum, reg_buf);
+ if (status != REG_VALID)
+ gdb_assert_not_reached ("No vpr register for MVE.");
+
+ /* Update the last 2 bytes of reg_buf with the buf value. */
+ memcpy (reg_buf+2, buf, 2);
+
+ regcache->raw_write (vpr_regnum, reg_buf);
+}
+
static void
arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
int regnum, const gdb_byte *buf)
@@ -8821,9 +8895,13 @@ arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
gdb_assert (regnum >= num_regs);
regnum -= num_regs;
- if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
+ if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32
+ && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 48)
+ || (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 40)))
/* Quad-precision register. */
arm_neon_quad_write (gdbarch, regcache, regnum - 32, buf);
+ else if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == 40)
+ arm_mve_pseudo_write (gdbarch, regcache, regnum - 40, buf);
else
{
/* Single-precision register. */
@@ -8921,6 +8999,11 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
register_remote_g_packet_guess (gdbarch,
ARM_CORE_REGS_SIZE + ARM_VFP2_REGS_SIZE,
tdesc);
+ /* M-profile plus MVE. */
+ tdesc = arm_read_mprofile_description (ARM_M_TYPE_MVE);
+ register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE
+ + ARM_VFP2_REGS_SIZE
+ + ARM_INT_REGISTER_SIZE, tdesc);
}
/* Otherwise we don't have a useful guess. */
@@ -8971,8 +9054,10 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdesc_arch_data_up tdesc_data;
int i;
bool is_m = false;
+ bool is_mve = false;
int vfp_register_count = 0;
bool have_vfp_pseudos = false, have_neon_pseudos = false;
+ bool have_mve_pseudos = false;
bool have_wmmx_registers = false;
bool have_neon = false;
bool have_fpa_registers = true;
@@ -9091,6 +9176,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (!tdesc_has_registers (tdesc)
&& (attr_arch == TAG_CPU_ARCH_V6_M
|| attr_arch == TAG_CPU_ARCH_V6S_M
+ || attr_arch == TAG_CPU_ARCH_V8_1M_MAIN
|| attr_profile == 'M'))
is_m = true;
#endif
@@ -9169,8 +9255,20 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
ARM_PC_REGNUM,
arm_pc_names);
if (is_m)
- valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
- ARM_PS_REGNUM, "xpsr");
+ {
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ ARM_PS_REGNUM, "xpsr");
+ feature = tdesc_find_feature (tdesc,"org.gnu.gdb.arm.m-profile-mve");
+
+ if (feature != NULL)
+ {
+ is_mve = true;
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ ARM_VPR_REGNUM, "vpr");
+ if (tdesc_unnumbered_register (feature, "p0") == 0)
+ have_mve_pseudos = true;
+ }
+ }
else
valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
ARM_PS_REGNUM, "cpsr");
@@ -9270,7 +9368,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
does not support that. */
feature = tdesc_find_feature (tdesc,
"org.gnu.gdb.arm.neon");
- if (feature != NULL)
+ /* If we have VFP and no NEON, check for MVE. MVE uses NEON pseudos
+ q0 to q7. */
+ if (feature != NULL || is_mve)
{
/* NEON requires 32 double-precision registers. */
if (i != 32)
@@ -9282,7 +9382,8 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (tdesc_unnumbered_register (feature, "q0") == 0)
have_neon_pseudos = true;
- have_neon = true;
+ if (!is_mve)
+ have_neon = true;
}
}
}
@@ -9332,6 +9433,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdep->vfp_register_count = vfp_register_count;
tdep->have_vfp_pseudos = have_vfp_pseudos;
tdep->have_neon_pseudos = have_neon_pseudos;
+ tdep->have_mve_pseudos = have_mve_pseudos;
tdep->have_neon = have_neon;
arm_register_g_packet_guesses (gdbarch);
@@ -9432,7 +9534,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
set_gdbarch_dwarf2_reg_to_regnum (gdbarch, arm_dwarf_reg_to_regnum);
set_gdbarch_register_sim_regno (gdbarch, arm_register_sim_regno);
- set_gdbarch_register_name (gdbarch, arm_register_name);
+ set_gdbarch_register_name (gdbarch, arm_pseudo_register_name);
/* Returning results. */
set_gdbarch_return_value (gdbarch, arm_return_value);
@@ -9509,8 +9611,11 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
little more care in numbering will be needed. */
int num_pseudos = 32;
- if (have_neon_pseudos)
+ if (have_neon_pseudos && !is_mve)
num_pseudos += 16;
+ /* q0-q7 neon pseduo registers + p0 mve pseduo register. */
+ else if (have_mve_pseudos && is_mve)
+ num_pseudos += 9;
set_gdbarch_num_pseudo_regs (gdbarch, num_pseudos);
set_gdbarch_pseudo_register_read (gdbarch, arm_pseudo_read);
set_gdbarch_pseudo_register_write (gdbarch, arm_pseudo_write);
@@ -9518,7 +9623,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (tdesc_data != nullptr)
{
- set_tdesc_pseudo_register_name (gdbarch, arm_register_name);
+ set_tdesc_pseudo_register_name (gdbarch, arm_pseudo_register_name);
tdesc_use_registers (gdbarch, tdesc, std::move (tdesc_data));
@@ -9564,6 +9669,8 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
(int) tdep->have_neon_pseudos);
fprintf_unfiltered (file, _("arm_dump_tdep: have_neon = %i\n"),
(int) tdep->have_neon);
+ fprintf_unfiltered (file, _("arm_dump_tdep: have_mve_pseudos = %i\n"),
+ (int) tdep->have_mve_pseudos);
fprintf_unfiltered (file, _("arm_dump_tdep: Lowest pc = 0x%lx\n"),
(unsigned long) tdep->lowest_pc);
}
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8bff27c940dec2c5a627459b343a33f3b4f886a1..8348565a6a0c2ba16049030e00e270d2c641ddaa 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -45329,6 +45329,9 @@ and @samp{xpsr}.
The @samp{org.gnu.gdb.arm.fpa} feature is optional. If present, it
should contain registers @samp{f0} through @samp{f7} and @samp{fps}.
+The @samp{org.gnu.gdb.arm.m-profile-mve} feature is optional. If present, it
+should contain register @samp{vpr}.
+
The @samp{org.gnu.gdb.xscale.iwmmxt} feature is optional. If present,
it should contain at least registers @samp{wR0} through @samp{wR15} and
@samp{wCGR0} through @samp{wCGR3}. The @samp{wCID}, @samp{wCon},
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 689603847a001ae0297558cd1fde1242b1c9800c..51db08fe9fd2e8197613b971b8f9fbe2b8daf0f3 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -208,6 +208,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
arm/arm-core.xml \
arm/arm-fpa.xml \
arm/arm-m-profile.xml \
+ arm/arm-m-profile-with-mve.xml \
arm/arm-m-profile-with-fpa.xml \
arm/arm-vfpv2.xml \
arm/arm-vfpv3.xml \
diff --git a/gdb/features/arm/arm-m-profile-with-mve.c b/gdb/features/arm/arm-m-profile-with-mve.c
new file mode 100644
index 0000000000000000000000000000000000000000..b23e77f7427b197a927d31cf1bc3a9f5a2354d4e
--- /dev/null
+++ b/gdb/features/arm/arm-m-profile-with-mve.c
@@ -0,0 +1,21 @@
+/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
+ Original: arm-m-profile-with-mve.xml */
+
+#include "gdbsupport/tdesc.h"
+
+static int
+create_feature_arm_arm_m_profile_with_mve (struct target_desc *result, long regnum)
+{
+ struct tdesc_feature *feature;
+
+ feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile-mve");
+ tdesc_type_with_fields *type_with_fields;
+ type_with_fields = tdesc_create_flags (feature, "vpr_reg", 4);
+ tdesc_add_bitfield (type_with_fields, "P0", 0, 15);
+ tdesc_add_bitfield (type_with_fields, "MASK01", 16, 19);
+ tdesc_add_bitfield (type_with_fields, "MASK23", 20, 23);
+ tdesc_add_bitfield (type_with_fields, "RES0", 24, 31);
+
+ tdesc_create_reg (feature, "vpr", regnum++, 1, NULL, 32, "vpr_reg");
+ return regnum;
+}
diff --git a/gdb/features/arm/arm-m-profile-with-mve.xml b/gdb/features/arm/arm-m-profile-with-mve.xml
new file mode 100644
index 0000000000000000000000000000000000000000..35b17351e0ca0c418c138c3eb711b4e54daac95c
--- /dev/null
+++ b/gdb/features/arm/arm-m-profile-with-mve.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2021 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.arm.m-profile-mve">
+ <flags id="vpr_reg" size="4">
+ <!-- ARMv8.1-M and MVE: Unprivileged and privileged Access. -->
+ <field name="P0" start="0" end="15"/>
+ <!-- ARMv8.1-M: Privileged Access only. -->
+ <field name="MASK01" start="16" end="19"/>
+ <!-- ARMv8.1-M: Privileged Access only. -->
+ <field name="MASK23" start="20" end="23"/>
+ <!-- ARMv8.1-M: Privileged Access only. -->
+ <field name="RES0" start="24" end="31"/>
+ </flags>
+ <reg name="vpr" bitsize="32" type="vpr_reg"/>
+</feature>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [GDB][PATCH] arm: Add support for mve vector predicate status and control register (vpr) and pseudo register p0.
2021-07-02 8:47 [GDB][PATCH] arm: Add support for mve vector predicate status and control register (vpr) and pseudo register p0 Srinath Parvathaneni
@ 2021-07-02 15:44 ` Alan Hayward
2021-07-05 10:39 ` Luis Machado
1 sibling, 0 replies; 4+ messages in thread
From: Alan Hayward @ 2021-07-02 15:44 UTC (permalink / raw)
To: Srinath Parvathaneni
Cc: gdb-patches\@sourceware.org, Peter Maydell, Luis Machado
> On 2 Jul 2021, at 09:47, Srinath Parvathaneni <Srinath.Parvathaneni@arm.com> wrote:
>
> Hello,
>
> This patch add support for mve vector predication status and control register (vpr) and pseudo register p0.
> Pseudo register p0 is the least significant bits of vpr and can be accessed as $p0, $vpr.p0.
> For more information about the register please refer to [1].
>
> [1] https://developer.arm.com/documentation/ddi0553/bn
>
> Built gdb for arm-none-eabi target and regtested.
>
> Ok for master?
It’s worth pointing out the testing for this patch. There is no real hardware available yet. In addition,
there is no support in QEMU, but QMEU is waiting for GDB so they can base their work off it.
(Added Peter on cc). So there is a chicken and egg situation we’ve seen before.
Given this patch adheres to the hardware spec, it’s a fairly boilerplate patch, and it doesn’t effect any
other configurations, then I’m inclined to say that’s all ok.
My only concern with the code is that all the regnum if statements are starting to get a little unwieldy.
But, given all the above statements, refactoring that in this patch I feel would be a mistake. I’d much
rather this simpler patch for now.
It all looks good to me, but it’d good to get another ok.
Alan.
>
> Regards,
> Srinath
>
> gdb/ChangeLog:
>
> 2021-07-02 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
>
> * arch/arm.c (arm_create_mprofile_target_description): Add a new case
> for m_type ARM_M_TYPE_MVE.
> (ARM_M_TYPE_MVE): Add m_type case to create feature profiles.
> * arch/arm.h (ARM_VPR_REGNUM): Add to enum gdb_regnum.
> (ARM_M_TYPE_MVE): Add to enum arm_m_profile_type.
> * arm-tdep.c (arm_register_type): Modify function to return
> builtin_int16 on receiving mve pseudo register p0.
> (arm_register_name): Rename to arm_pseudo_register_name.
> (arm_pseudo_register_name): Renamed arm_register_name and add a case
> for mve pseudo reigster p0.
> (arm_mve_pseudo_read): Define function to read value from VPR.P0.
> (arm_pseudo_read): Add case for mve pseudo reigster p0.
> (arm_mve_pseudo_write): Define function to write value to VPR.P0.
> (arm_pseudo_write): Add case for mve pseudo reigster p0.
> (arm_register_g_packet_guesses): Allocate memory for registers of
> ARM_M_TYPE_MVE m_type.
> (arm_gdbarch_init): Add support for new feature arm.m-profile-with-mve
> and new reigster ARM_VPR_REGNUM.
> (arm_dump_tdep): Print arm_dump_tdep when mve pseudo is present.
> * arm-tdep.h (struct gdbarch_tdep): Define have_mve_pseudos.
> * features/Makefile (arm/arm-m-profile-with-mve.xml): Add m-profile
> mve feature XML file.
> * features/arm/arm-m-profile-with-mve.c: Renegerated file.
> * features/arm/arm-m-profile-with-mve.xml: New file.
>
> gdb/doc/ChangeLog:
>
> 2021-07-02 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
>
> * gdb.texinfo (org.gnu.gdb.arm.m-profile-mve): Add mve feature entry.
>
>
>
> ############### Attachment also inlined for ease of reply ###############
>
>
> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
> index b0eb2ae445f2c716dd21dbbb62b87ed678d93722..8e41fa73d77dc1caa4749ae9d82cbe6bc615ec9c 100644
> --- a/gdb/arch/arm.h
> +++ b/gdb/arch/arm.h
> @@ -49,6 +49,7 @@ enum gdb_regnum {
> ARM_D0_REGNUM, /* VFP double-precision registers. */
> ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
> ARM_FPSCR_REGNUM,
> + ARM_VPR_REGNUM,
>
> ARM_NUM_REGS,
>
> @@ -83,6 +84,7 @@ enum arm_m_profile_type {
> ARM_M_TYPE_M_PROFILE,
> ARM_M_TYPE_VFP_D16,
> ARM_M_TYPE_WITH_FPA,
> + ARM_M_TYPE_MVE,
> ARM_M_TYPE_INVALID
> };
>
> diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
> index faa2b4fbd4b21588d46c5515b96a838d9ad94553..25065f2a57e774c38bf2384bf29aa62ef60eed04 100644
> --- a/gdb/arch/arm.c
> +++ b/gdb/arch/arm.c
> @@ -27,6 +27,7 @@
> #include "../features/arm/xscale-iwmmxt.c"
> #include "../features/arm/arm-m-profile.c"
> #include "../features/arm/arm-m-profile-with-fpa.c"
> +#include "../features/arm/arm-m-profile-with-mve.c"
>
> /* See arm.h. */
>
> @@ -439,6 +440,12 @@ arm_create_mprofile_target_description (arm_m_profile_type m_type)
> regnum = create_feature_arm_arm_m_profile_with_fpa (tdesc, regnum);
> break;
>
> + case ARM_M_TYPE_MVE:
> + regnum = create_feature_arm_arm_m_profile (tdesc, regnum);
> + regnum = create_feature_arm_arm_vfpv2 (tdesc, regnum);
> + regnum = create_feature_arm_arm_m_profile_with_mve (tdesc, regnum);
> + break;
> +
> default:
> error (_("Invalid Arm M type: %d"), m_type);
> }
> diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
> index 629c3b8ac685f411fe7eba45eb37ded3ea9d2a82..22b1a6ea0a02984a122ddc19f300d46f05c056dc 100644
> --- a/gdb/arm-tdep.h
> +++ b/gdb/arm-tdep.h
> @@ -106,6 +106,10 @@ struct gdbarch_tdep
> NEON registers? Requires
> have_vfp_pseudos. */
> bool have_neon; /* Do we have a NEON unit? */
> + bool have_mve_pseudos; /* We are synthesizing the p0 register from VPR
> + register. For this we need have_vfp_pseudos
> + and have_neon_pseudos true, so that p0 is
> + added to the pseudo register list. */
>
> bool is_m; /* Does the target follow the "M" profile. */
> CORE_ADDR lowest_pc; /* Lowest address at which instructions
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index a214f22d7ad10740c67509c4018b4494c5e7f7fe..7f8f7bc2ca3852f23b512a7ab1ef1aa8a1d4b49e 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -4089,10 +4089,15 @@ arm_register_type (struct gdbarch *gdbarch, int regnum)
> && regnum >= num_regs && regnum < num_regs + 32)
> return builtin_type (gdbarch)->builtin_float;
>
> - if (gdbarch_tdep (gdbarch)->have_neon_pseudos
> - && regnum >= num_regs + 32 && regnum < num_regs + 32 + 16)
> + if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= num_regs + 32
> + && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < num_regs + 48)
> + || (gdbarch_tdep (gdbarch)->have_mve_pseudos
> + && regnum < num_regs + 40)))
> return arm_neon_quad_type (gdbarch);
>
> + if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == num_regs + 40)
> + return builtin_type (gdbarch)->builtin_int16;
> +
> /* If the target description has register information, we are only
> in this function so that we can override the types of
> double-precision registers for NEON. */
> @@ -8581,7 +8586,7 @@ show_disassembly_style_sfunc (struct ui_file *file, int from_tty,
>
> /* Return the ARM register name corresponding to register I. */
> static const char *
> -arm_register_name (struct gdbarch *gdbarch, int i)
> +arm_pseudo_register_name (struct gdbarch *gdbarch, int i)
> {
> const int num_regs = gdbarch_num_regs (gdbarch);
>
> @@ -8598,8 +8603,9 @@ arm_register_name (struct gdbarch *gdbarch, int i)
> return vfp_pseudo_names[i - num_regs];
> }
>
> - if (gdbarch_tdep (gdbarch)->have_neon_pseudos
> - && i >= num_regs + 32 && i < num_regs + 32 + 16)
> + if (gdbarch_tdep (gdbarch)->have_neon_pseudos && i >= num_regs + 32
> + && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && i < num_regs + 48)
> + || (gdbarch_tdep (gdbarch)->have_mve_pseudos && i < num_regs + 40)))
> {
> static const char *const neon_pseudo_names[] = {
> "q0", "q1", "q2", "q3", "q4", "q5", "q6", "q7",
> @@ -8608,6 +8614,12 @@ arm_register_name (struct gdbarch *gdbarch, int i)
>
> return neon_pseudo_names[i - num_regs - 32];
> }
> + if (gdbarch_tdep (gdbarch)->have_mve_pseudos && i == num_regs + 40)
> + {
> + static const char *const mve_pseudo_names[] = {"p0"};
> +
> + return mve_pseudo_names[i - num_regs - 40];
> + }
>
> if (i >= ARRAY_SIZE (arm_register_names))
> /* These registers are only supported on targets which supply
> @@ -8741,6 +8753,35 @@ arm_neon_quad_read (struct gdbarch *gdbarch, readable_regcache *regcache,
> return REG_VALID;
> }
>
> +/* Store pseudo register P0 to BUF, P0 is least significant 16 bits of
> + Armv8.1-M VPR register. */
> +static enum register_status
> +arm_mve_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
> + int regnum, gdb_byte *buf)
> +{
> + char name_buf[4];
> + gdb_byte reg_buf[4];
> + int vpr_regnum;
> + enum register_status status;
> +
> + if (regnum == 0)
> + xsnprintf (name_buf, sizeof (name_buf), "vpr");
> + else
> + gdb_assert_not_reached ("wrong pseudo regnum for MVE.");
> +
> + vpr_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
> + strlen (name_buf));
> + /* Read the actual value in the VPR register. */
> + status = regcache->raw_read (vpr_regnum, reg_buf);
> + if (status != REG_VALID)
> + gdb_assert_not_reached ("No vpr register for MVE.");
> +
> + /* Copy only the last 2 bytes of reg_buf to buf. */
> + memcpy (buf, reg_buf + 2, 2);
> +
> + return REG_VALID;
> +}
> +
> static enum register_status
> arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
> int regnum, gdb_byte *buf)
> @@ -8753,9 +8794,13 @@ arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
> gdb_assert (regnum >= num_regs);
> regnum -= num_regs;
>
> - if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
> + if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32
> + && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 48)
> + || (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 40)))
> /* Quad-precision register. */
> return arm_neon_quad_read (gdbarch, regcache, regnum - 32, buf);
> + else if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == 40)
> + return arm_mve_pseudo_read (gdbarch, regcache, regnum - 40, buf);
> else
> {
> enum register_status status;
> @@ -8809,6 +8854,35 @@ arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache,
> regcache->raw_write (double_regnum + 1, buf + offset);
> }
>
> +/* BUF needs to be stored to pseudo register P0, which is
> + least significant 16 bits of Armv8.1-M VPR register. */
> +static void
> +arm_mve_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
> + int regnum, const gdb_byte *buf)
> +{
> + char name_buf[4];
> + gdb_byte reg_buf[4];
> + int vpr_regnum;
> + enum register_status status;
> +
> + if (regnum == 0)
> + xsnprintf (name_buf, sizeof (name_buf), "vpr");
> + else
> + gdb_assert_not_reached ("wrong pseudo regnum for MVE.");
> +
> + vpr_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
> + strlen (name_buf));
> + /* Read the actual value in the VPR register. */
> + status = regcache->raw_read (vpr_regnum, reg_buf);
> + if (status != REG_VALID)
> + gdb_assert_not_reached ("No vpr register for MVE.");
> +
> + /* Update the last 2 bytes of reg_buf with the buf value. */
> + memcpy (reg_buf+2, buf, 2);
> +
> + regcache->raw_write (vpr_regnum, reg_buf);
> +}
> +
> static void
> arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
> int regnum, const gdb_byte *buf)
> @@ -8821,9 +8895,13 @@ arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
> gdb_assert (regnum >= num_regs);
> regnum -= num_regs;
>
> - if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
> + if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32
> + && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 48)
> + || (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 40)))
> /* Quad-precision register. */
> arm_neon_quad_write (gdbarch, regcache, regnum - 32, buf);
> + else if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == 40)
> + arm_mve_pseudo_write (gdbarch, regcache, regnum - 40, buf);
> else
> {
> /* Single-precision register. */
> @@ -8921,6 +8999,11 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
> register_remote_g_packet_guess (gdbarch,
> ARM_CORE_REGS_SIZE + ARM_VFP2_REGS_SIZE,
> tdesc);
> + /* M-profile plus MVE. */
> + tdesc = arm_read_mprofile_description (ARM_M_TYPE_MVE);
> + register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE
> + + ARM_VFP2_REGS_SIZE
> + + ARM_INT_REGISTER_SIZE, tdesc);
> }
>
> /* Otherwise we don't have a useful guess. */
> @@ -8971,8 +9054,10 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> tdesc_arch_data_up tdesc_data;
> int i;
> bool is_m = false;
> + bool is_mve = false;
> int vfp_register_count = 0;
> bool have_vfp_pseudos = false, have_neon_pseudos = false;
> + bool have_mve_pseudos = false;
> bool have_wmmx_registers = false;
> bool have_neon = false;
> bool have_fpa_registers = true;
> @@ -9091,6 +9176,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> if (!tdesc_has_registers (tdesc)
> && (attr_arch == TAG_CPU_ARCH_V6_M
> || attr_arch == TAG_CPU_ARCH_V6S_M
> + || attr_arch == TAG_CPU_ARCH_V8_1M_MAIN
> || attr_profile == 'M'))
> is_m = true;
> #endif
> @@ -9169,8 +9255,20 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> ARM_PC_REGNUM,
> arm_pc_names);
> if (is_m)
> - valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
> - ARM_PS_REGNUM, "xpsr");
> + {
> + valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
> + ARM_PS_REGNUM, "xpsr");
> + feature = tdesc_find_feature (tdesc,"org.gnu.gdb.arm.m-profile-mve");
> +
> + if (feature != NULL)
> + {
> + is_mve = true;
> + valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
> + ARM_VPR_REGNUM, "vpr");
> + if (tdesc_unnumbered_register (feature, "p0") == 0)
> + have_mve_pseudos = true;
> + }
> + }
> else
> valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
> ARM_PS_REGNUM, "cpsr");
> @@ -9270,7 +9368,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> does not support that. */
> feature = tdesc_find_feature (tdesc,
> "org.gnu.gdb.arm.neon");
> - if (feature != NULL)
> + /* If we have VFP and no NEON, check for MVE. MVE uses NEON pseudos
> + q0 to q7. */
> + if (feature != NULL || is_mve)
> {
> /* NEON requires 32 double-precision registers. */
> if (i != 32)
> @@ -9282,7 +9382,8 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> if (tdesc_unnumbered_register (feature, "q0") == 0)
> have_neon_pseudos = true;
>
> - have_neon = true;
> + if (!is_mve)
> + have_neon = true;
> }
> }
> }
> @@ -9332,6 +9433,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> tdep->vfp_register_count = vfp_register_count;
> tdep->have_vfp_pseudos = have_vfp_pseudos;
> tdep->have_neon_pseudos = have_neon_pseudos;
> + tdep->have_mve_pseudos = have_mve_pseudos;
> tdep->have_neon = have_neon;
>
> arm_register_g_packet_guesses (gdbarch);
> @@ -9432,7 +9534,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, arm_dwarf_reg_to_regnum);
> set_gdbarch_register_sim_regno (gdbarch, arm_register_sim_regno);
>
> - set_gdbarch_register_name (gdbarch, arm_register_name);
> + set_gdbarch_register_name (gdbarch, arm_pseudo_register_name);
>
> /* Returning results. */
> set_gdbarch_return_value (gdbarch, arm_return_value);
> @@ -9509,8 +9611,11 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> little more care in numbering will be needed. */
>
> int num_pseudos = 32;
> - if (have_neon_pseudos)
> + if (have_neon_pseudos && !is_mve)
> num_pseudos += 16;
> + /* q0-q7 neon pseduo registers + p0 mve pseduo register. */
> + else if (have_mve_pseudos && is_mve)
> + num_pseudos += 9;
> set_gdbarch_num_pseudo_regs (gdbarch, num_pseudos);
> set_gdbarch_pseudo_register_read (gdbarch, arm_pseudo_read);
> set_gdbarch_pseudo_register_write (gdbarch, arm_pseudo_write);
> @@ -9518,7 +9623,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>
> if (tdesc_data != nullptr)
> {
> - set_tdesc_pseudo_register_name (gdbarch, arm_register_name);
> + set_tdesc_pseudo_register_name (gdbarch, arm_pseudo_register_name);
>
> tdesc_use_registers (gdbarch, tdesc, std::move (tdesc_data));
>
> @@ -9564,6 +9669,8 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
> (int) tdep->have_neon_pseudos);
> fprintf_unfiltered (file, _("arm_dump_tdep: have_neon = %i\n"),
> (int) tdep->have_neon);
> + fprintf_unfiltered (file, _("arm_dump_tdep: have_mve_pseudos = %i\n"),
> + (int) tdep->have_mve_pseudos);
> fprintf_unfiltered (file, _("arm_dump_tdep: Lowest pc = 0x%lx\n"),
> (unsigned long) tdep->lowest_pc);
> }
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 8bff27c940dec2c5a627459b343a33f3b4f886a1..8348565a6a0c2ba16049030e00e270d2c641ddaa 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -45329,6 +45329,9 @@ and @samp{xpsr}.
> The @samp{org.gnu.gdb.arm.fpa} feature is optional. If present, it
> should contain registers @samp{f0} through @samp{f7} and @samp{fps}.
>
> +The @samp{org.gnu.gdb.arm.m-profile-mve} feature is optional. If present, it
> +should contain register @samp{vpr}.
> +
> The @samp{org.gnu.gdb.xscale.iwmmxt} feature is optional. If present,
> it should contain at least registers @samp{wR0} through @samp{wR15} and
> @samp{wCGR0} through @samp{wCGR3}. The @samp{wCID}, @samp{wCon},
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index 689603847a001ae0297558cd1fde1242b1c9800c..51db08fe9fd2e8197613b971b8f9fbe2b8daf0f3 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -208,6 +208,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
> arm/arm-core.xml \
> arm/arm-fpa.xml \
> arm/arm-m-profile.xml \
> + arm/arm-m-profile-with-mve.xml \
> arm/arm-m-profile-with-fpa.xml \
> arm/arm-vfpv2.xml \
> arm/arm-vfpv3.xml \
> diff --git a/gdb/features/arm/arm-m-profile-with-mve.c b/gdb/features/arm/arm-m-profile-with-mve.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b23e77f7427b197a927d31cf1bc3a9f5a2354d4e
> --- /dev/null
> +++ b/gdb/features/arm/arm-m-profile-with-mve.c
> @@ -0,0 +1,21 @@
> +/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
> + Original: arm-m-profile-with-mve.xml */
> +
> +#include "gdbsupport/tdesc.h"
> +
> +static int
> +create_feature_arm_arm_m_profile_with_mve (struct target_desc *result, long regnum)
> +{
> + struct tdesc_feature *feature;
> +
> + feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile-mve");
> + tdesc_type_with_fields *type_with_fields;
> + type_with_fields = tdesc_create_flags (feature, "vpr_reg", 4);
> + tdesc_add_bitfield (type_with_fields, "P0", 0, 15);
> + tdesc_add_bitfield (type_with_fields, "MASK01", 16, 19);
> + tdesc_add_bitfield (type_with_fields, "MASK23", 20, 23);
> + tdesc_add_bitfield (type_with_fields, "RES0", 24, 31);
> +
> + tdesc_create_reg (feature, "vpr", regnum++, 1, NULL, 32, "vpr_reg");
> + return regnum;
> +}
> diff --git a/gdb/features/arm/arm-m-profile-with-mve.xml b/gdb/features/arm/arm-m-profile-with-mve.xml
> new file mode 100644
> index 0000000000000000000000000000000000000000..35b17351e0ca0c418c138c3eb711b4e54daac95c
> --- /dev/null
> +++ b/gdb/features/arm/arm-m-profile-with-mve.xml
> @@ -0,0 +1,21 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2021 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.arm.m-profile-mve">
> + <flags id="vpr_reg" size="4">
> + <!-- ARMv8.1-M and MVE: Unprivileged and privileged Access. -->
> + <field name="P0" start="0" end="15"/>
> + <!-- ARMv8.1-M: Privileged Access only. -->
> + <field name="MASK01" start="16" end="19"/>
> + <!-- ARMv8.1-M: Privileged Access only. -->
> + <field name="MASK23" start="20" end="23"/>
> + <!-- ARMv8.1-M: Privileged Access only. -->
> + <field name="RES0" start="24" end="31"/>
> + </flags>
> + <reg name="vpr" bitsize="32" type="vpr_reg"/>
> +</feature>
>
> <rb14189.patch>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [GDB][PATCH] arm: Add support for mve vector predicate status and control register (vpr) and pseudo register p0.
2021-07-02 8:47 [GDB][PATCH] arm: Add support for mve vector predicate status and control register (vpr) and pseudo register p0 Srinath Parvathaneni
2021-07-02 15:44 ` Alan Hayward
@ 2021-07-05 10:39 ` Luis Machado
2021-07-05 11:03 ` Peter Maydell
1 sibling, 1 reply; 4+ messages in thread
From: Luis Machado @ 2021-07-05 10:39 UTC (permalink / raw)
To: Srinath Parvathaneni, gdb-patches, Peter Maydell, Alan Hayward
Hi Srinath,
On 7/2/21 5:47 AM, Srinath Parvathaneni via Gdb-patches wrote:
> Hello,
>
> This patch add support for mve vector predication status and control register (vpr) and pseudo register p0.
> Pseudo register p0 is the least significant bits of vpr and can be accessed as $p0, $vpr.p0.
> For more information about the register please refer to [1].
>
> [1] https://developer.arm.com/documentation/ddi0553/bn
>
> Built gdb for arm-none-eabi target and regtested.
>
> Ok for master?
>
> Regards,
> Srinath
>
> gdb/ChangeLog:
>
> 2021-07-02 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
>
> * arch/arm.c (arm_create_mprofile_target_description): Add a new case
> for m_type ARM_M_TYPE_MVE.
> (ARM_M_TYPE_MVE): Add m_type case to create feature profiles.
> * arch/arm.h (ARM_VPR_REGNUM): Add to enum gdb_regnum.
> (ARM_M_TYPE_MVE): Add to enum arm_m_profile_type.
> * arm-tdep.c (arm_register_type): Modify function to return
> builtin_int16 on receiving mve pseudo register p0.
> (arm_register_name): Rename to arm_pseudo_register_name.
> (arm_pseudo_register_name): Renamed arm_register_name and add a case
> for mve pseudo reigster p0.
> (arm_mve_pseudo_read): Define function to read value from VPR.P0.
> (arm_pseudo_read): Add case for mve pseudo reigster p0.
> (arm_mve_pseudo_write): Define function to write value to VPR.P0.
> (arm_pseudo_write): Add case for mve pseudo reigster p0.
> (arm_register_g_packet_guesses): Allocate memory for registers of
> ARM_M_TYPE_MVE m_type.
> (arm_gdbarch_init): Add support for new feature arm.m-profile-with-mve
> and new reigster ARM_VPR_REGNUM.
> (arm_dump_tdep): Print arm_dump_tdep when mve pseudo is present.
> * arm-tdep.h (struct gdbarch_tdep): Define have_mve_pseudos.
> * features/Makefile (arm/arm-m-profile-with-mve.xml): Add m-profile
> mve feature XML file.
> * features/arm/arm-m-profile-with-mve.c: Renegerated file.
> * features/arm/arm-m-profile-with-mve.xml: New file.
>
> gdb/doc/ChangeLog:
>
> 2021-07-02 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
>
> * gdb.texinfo (org.gnu.gdb.arm.m-profile-mve): Add mve feature entry.
>
From GDB 11 onwards, we no longer require ChangeLog entries. Feel free
to drop it in favor of a more descriptive commit message instead.
>
>
> ############### Attachment also inlined for ease of reply ###############
>
>
> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
> index b0eb2ae445f2c716dd21dbbb62b87ed678d93722..8e41fa73d77dc1caa4749ae9d82cbe6bc615ec9c 100644
> --- a/gdb/arch/arm.h
> +++ b/gdb/arch/arm.h
> @@ -49,6 +49,7 @@ enum gdb_regnum {
> ARM_D0_REGNUM, /* VFP double-precision registers. */
> ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
> ARM_FPSCR_REGNUM,
> + ARM_VPR_REGNUM,
>
> ARM_NUM_REGS,
>
The number of REGNUM constants keeps increasing, it would be nice to
move the register number mechanisms to use dynamic numbers. Just
something to keep in mind.
Given the arm target doesn't change that much, I think it should be ok
to stick to this mechanism for this change.
> @@ -83,6 +84,7 @@ enum arm_m_profile_type {
> ARM_M_TYPE_M_PROFILE,
> ARM_M_TYPE_VFP_D16,
> ARM_M_TYPE_WITH_FPA,
> + ARM_M_TYPE_MVE,
> ARM_M_TYPE_INVALID
> };
> > diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
> index faa2b4fbd4b21588d46c5515b96a838d9ad94553..25065f2a57e774c38bf2384bf29aa62ef60eed04 100644
> --- a/gdb/arch/arm.c
> +++ b/gdb/arch/arm.c
> @@ -27,6 +27,7 @@
> #include "../features/arm/xscale-iwmmxt.c"
> #include "../features/arm/arm-m-profile.c"
> #include "../features/arm/arm-m-profile-with-fpa.c"
> +#include "../features/arm/arm-m-profile-with-mve.c"
>
> /* See arm.h. */
>
> @@ -439,6 +440,12 @@ arm_create_mprofile_target_description (arm_m_profile_type m_type)
> regnum = create_feature_arm_arm_m_profile_with_fpa (tdesc, regnum);
> break;
>
> + case ARM_M_TYPE_MVE:
> + regnum = create_feature_arm_arm_m_profile (tdesc, regnum);
> + regnum = create_feature_arm_arm_vfpv2 (tdesc, regnum);
> + regnum = create_feature_arm_arm_m_profile_with_mve (tdesc, regnum);
> + break;
> +
> default:
> error (_("Invalid Arm M type: %d"), m_type);
> }
> diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
> index 629c3b8ac685f411fe7eba45eb37ded3ea9d2a82..22b1a6ea0a02984a122ddc19f300d46f05c056dc 100644
> --- a/gdb/arm-tdep.h
> +++ b/gdb/arm-tdep.h
> @@ -106,6 +106,10 @@ struct gdbarch_tdep
> NEON registers? Requires
> have_vfp_pseudos. */
> bool have_neon; /* Do we have a NEON unit? */
> + bool have_mve_pseudos; /* We are synthesizing the p0 register from VPR
> + register. For this we need have_vfp_pseudos > + and have_neon_pseudos true, so that p0 is
Capitalize HAVE_VFP_PSEUDOS and HAVE_NEON_PSEUDOS to make it easier to
spot the fields.
> + added to the pseudo register list. */
>
> bool is_m; /* Does the target follow the "M" profile. */
> CORE_ADDR lowest_pc; /* Lowest address at which instructions
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index a214f22d7ad10740c67509c4018b4494c5e7f7fe..7f8f7bc2ca3852f23b512a7ab1ef1aa8a1d4b49e 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -4089,10 +4089,15 @@ arm_register_type (struct gdbarch *gdbarch, int regnum)
> && regnum >= num_regs && regnum < num_regs + 32)
> return builtin_type (gdbarch)->builtin_float;
>
> - if (gdbarch_tdep (gdbarch)->have_neon_pseudos
> - && regnum >= num_regs + 32 && regnum < num_regs + 32 + 16)
> + if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= num_regs + 32
> + && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < num_regs + 48)
> + || (gdbarch_tdep (gdbarch)->have_mve_pseudos
> + && regnum < num_regs + 40)))
I see these checks have been used more than once. Can we turn them into
a predicate function that returns true/false? That would make it easier
to understand what the checks are doing.
> return arm_neon_quad_type (gdbarch);
>
> + if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == num_regs + 40)
> + return builtin_type (gdbarch)->builtin_int16;
> +
> /* If the target description has register information, we are only
> in this function so that we can override the types of
> double-precision registers for NEON. */
> @@ -8581,7 +8586,7 @@ show_disassembly_style_sfunc (struct ui_file *file, int from_tty,
> \f
> /* Return the ARM register name corresponding to register I. */
> static const char *
> -arm_register_name (struct gdbarch *gdbarch, int i)
> +arm_pseudo_register_name (struct gdbarch *gdbarch, int i)
> {
> const int num_regs = gdbarch_num_regs (gdbarch);
>
> @@ -8598,8 +8603,9 @@ arm_register_name (struct gdbarch *gdbarch, int i)
> return vfp_pseudo_names[i - num_regs];
> }
>
> - if (gdbarch_tdep (gdbarch)->have_neon_pseudos
> - && i >= num_regs + 32 && i < num_regs + 32 + 16)
> + if (gdbarch_tdep (gdbarch)->have_neon_pseudos && i >= num_regs + 32
> + && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && i < num_regs + 48)
> + || (gdbarch_tdep (gdbarch)->have_mve_pseudos && i < num_regs + 40)))
Here for example. It would be nice to just call a predicate function
instead of using this big condition.
> {
> static const char *const neon_pseudo_names[] = {
> "q0", "q1", "q2", "q3", "q4", "q5", "q6", "q7",
> @@ -8608,6 +8614,12 @@ arm_register_name (struct gdbarch *gdbarch, int i)
>
> return neon_pseudo_names[i - num_regs - 32];
> }
> + if (gdbarch_tdep (gdbarch)->have_mve_pseudos && i == num_regs + 40)
We could turn the above conditional into a predicate function as well.
> + {
> + static const char *const mve_pseudo_names[] = {"p0"};
> +
> + return mve_pseudo_names[i - num_regs - 40];
> + }
Why not return "p0" directly instead of indexing an array of 1 element?
>
> if (i >= ARRAY_SIZE (arm_register_names))
> /* These registers are only supported on targets which supply
> @@ -8741,6 +8753,35 @@ arm_neon_quad_read (struct gdbarch *gdbarch, readable_regcache *regcache,
> return REG_VALID;
> }
>
> +/* Store pseudo register P0 to BUF, P0 is least significant 16 bits of
> + Armv8.1-M VPR register. */
> +static enum register_status
> +arm_mve_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
> + int regnum, gdb_byte *buf)
New line between function comment and function declaration.
> +{
> + char name_buf[4];
> + gdb_byte reg_buf[4];
> + int vpr_regnum;
> + enum register_status status;
> +
> + if (regnum == 0)
> + xsnprintf (name_buf, sizeof (name_buf), "vpr");
Why not use a constant char * with value "vpr" instead?
> + else
> + gdb_assert_not_reached ("wrong pseudo regnum for MVE.");
> +
It looks cleaner to have the assertion at the beginning of the function
instead:
gdb_assert (regnum == 0 && "Wrong pseudo regnum for MVE.");
Or this:
if (regnum != 0)
gdb_assert_not_reached ("Wrong pseudo regnum for MVE.");
> + vpr_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
> + strlen (name_buf));
> + /* Read the actual value in the VPR register. */
> + status = regcache->raw_read (vpr_regnum, reg_buf);
> + if (status != REG_VALID)
> + gdb_assert_not_reached ("No vpr register for MVE.");
Is it safe to have this? If the register exists but the target failed to
read its contents, it will be unavailable, which is fine. We shouldn't
assert in that case. GDB will show the contents as "unavailable".
> +
> + /* Copy only the last 2 bytes of reg_buf to buf. */
> + memcpy (buf, reg_buf + 2, 2);
> +
> + return REG_VALID;
> +}
> +
> static enum register_status
> arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
> int regnum, gdb_byte *buf)
> @@ -8753,9 +8794,13 @@ arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
> gdb_assert (regnum >= num_regs);
> regnum -= num_regs;
>
> - if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
> + if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32
> + && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 48)
> + || (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 40)))
Turn this conditional into a predicate function.
> /* Quad-precision register. */
> return arm_neon_quad_read (gdbarch, regcache, regnum - 32, buf);
> + else if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == 40)
Turn this conditional into a predicate function as well.
> + return arm_mve_pseudo_read (gdbarch, regcache, regnum - 40, buf);
> else
> {
> enum register_status status;
> @@ -8809,6 +8854,35 @@ arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache,
> regcache->raw_write (double_regnum + 1, buf + offset);
> }
>
> +/* BUF needs to be stored to pseudo register P0, which is
> + least significant 16 bits of Armv8.1-M VPR register. */
> +static void
> +arm_mve_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
> + int regnum, const gdb_byte *buf)
New line between function comment and function declaration.
> +{
> + char name_buf[4];
> + gdb_byte reg_buf[4];
> + int vpr_regnum;
> + enum register_status status;
> +
> + if (regnum == 0)
> + xsnprintf (name_buf, sizeof (name_buf), "vpr");
Same comment about using a const char * for the "vpr" name.
> + else
> + gdb_assert_not_reached ("wrong pseudo regnum for MVE.");
Same comment about the assertion being at the top of the function instead.
> +
> + vpr_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
> + strlen (name_buf));
> + /* Read the actual value in the VPR register. */
> + status = regcache->raw_read (vpr_regnum, reg_buf);
> + if (status != REG_VALID)
> + gdb_assert_not_reached ("No vpr register for MVE.");
Same comment about not asserting when a register exists but is unavailable.
> +
> + /* Update the last 2 bytes of reg_buf with the buf value. */
> + memcpy (reg_buf+2, buf, 2);
> +
> + regcache->raw_write (vpr_regnum, reg_buf);
> +}
> +
> static void
> arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
> int regnum, const gdb_byte *buf)
> @@ -8821,9 +8895,13 @@ arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
> gdb_assert (regnum >= num_regs);
> regnum -= num_regs;
>
> - if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
> + if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32
> + && ((!gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 48)
> + || (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum < 40)))
Turn the conditional into a predicate function.
> /* Quad-precision register. */
> arm_neon_quad_write (gdbarch, regcache, regnum - 32, buf);
> + else if (gdbarch_tdep (gdbarch)->have_mve_pseudos && regnum == 40)
Turn the conditional into a predicate function.
> + arm_mve_pseudo_write (gdbarch, regcache, regnum - 40, buf);
> else
> {
> /* Single-precision register. */
> @@ -8921,6 +8999,11 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
> register_remote_g_packet_guess (gdbarch,
> ARM_CORE_REGS_SIZE + ARM_VFP2_REGS_SIZE,
> tdesc);
> + /* M-profile plus MVE. */
> + tdesc = arm_read_mprofile_description (ARM_M_TYPE_MVE);
> + register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE
> + + ARM_VFP2_REGS_SIZE
> + + ARM_INT_REGISTER_SIZE, tdesc);
register_remote_g_packet_guess is meant for backwards compatibility. Is
this needed for MVE given it is a new feature?
> }
>
> /* Otherwise we don't have a useful guess. */
> @@ -8971,8 +9054,10 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> tdesc_arch_data_up tdesc_data;
> int i;
> bool is_m = false;
> + bool is_mve = false;
> int vfp_register_count = 0;
> bool have_vfp_pseudos = false, have_neon_pseudos = false;
> + bool have_mve_pseudos = false;
> bool have_wmmx_registers = false;
> bool have_neon = false;
> bool have_fpa_registers = true;
> @@ -9091,6 +9176,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> if (!tdesc_has_registers (tdesc)
> && (attr_arch == TAG_CPU_ARCH_V6_M
> || attr_arch == TAG_CPU_ARCH_V6S_M
> + || attr_arch == TAG_CPU_ARCH_V8_1M_MAIN
> || attr_profile == 'M'))
> is_m = true;
> #endif
> @@ -9169,8 +9255,20 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> ARM_PC_REGNUM,
> arm_pc_names);
> if (is_m)
> - valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
> - ARM_PS_REGNUM, "xpsr");
> + {
> + valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
> + ARM_PS_REGNUM, "xpsr");
> + feature = tdesc_find_feature (tdesc,"org.gnu.gdb.arm.m-profile-mve");
> +
> + if (feature != NULL)
Please drop all uses of NULL and use nullptr instead. You can ignore
code you didn't touch, but new code should use nullptr.
> + {
> + is_mve = true;
> + valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
> + ARM_VPR_REGNUM, "vpr");
> + if (tdesc_unnumbered_register (feature, "p0") == 0)
> + have_mve_pseudos = true;
> + }
> + }
> else
> valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
> ARM_PS_REGNUM, "cpsr");
> @@ -9270,7 +9368,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> does not support that. */
> feature = tdesc_find_feature (tdesc,
> "org.gnu.gdb.arm.neon");
> - if (feature != NULL)
> + /* If we have VFP and no NEON, check for MVE. MVE uses NEON pseudos
> + q0 to q7. */
> + if (feature != NULL || is_mve)
> {
> /* NEON requires 32 double-precision registers. */
> if (i != 32)
> @@ -9282,7 +9382,8 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> if (tdesc_unnumbered_register (feature, "q0") == 0)
> have_neon_pseudos = true;
>
> - have_neon = true;
> + if (!is_mve)
> + have_neon = true;
> }
> }
> }
> @@ -9332,6 +9433,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> tdep->vfp_register_count = vfp_register_count;
> tdep->have_vfp_pseudos = have_vfp_pseudos;
> tdep->have_neon_pseudos = have_neon_pseudos;
> + tdep->have_mve_pseudos = have_mve_pseudos;
> tdep->have_neon = have_neon;
>
> arm_register_g_packet_guesses (gdbarch);
> @@ -9432,7 +9534,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, arm_dwarf_reg_to_regnum);
> set_gdbarch_register_sim_regno (gdbarch, arm_register_sim_regno);
>
> - set_gdbarch_register_name (gdbarch, arm_register_name);
> + set_gdbarch_register_name (gdbarch, arm_pseudo_register_name);
>
> /* Returning results. */
> set_gdbarch_return_value (gdbarch, arm_return_value);
> @@ -9509,8 +9611,11 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> little more care in numbering will be needed. */
>
> int num_pseudos = 32;
> - if (have_neon_pseudos)
> + if (have_neon_pseudos && !is_mve)
> num_pseudos += 16;
> + /* q0-q7 neon pseduo registers + p0 mve pseduo register. */
Typos... pseduo -> pseudo
> + else if (have_mve_pseudos && is_mve)
> + num_pseudos += 9;
Can we turn this literal constant into a meaningful named constant
instead? That will make it easier to know which "9" pseudos we're adding.
> set_gdbarch_num_pseudo_regs (gdbarch, num_pseudos);
> set_gdbarch_pseudo_register_read (gdbarch, arm_pseudo_read);
> set_gdbarch_pseudo_register_write (gdbarch, arm_pseudo_write);
> @@ -9518,7 +9623,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>
> if (tdesc_data != nullptr)
> {
> - set_tdesc_pseudo_register_name (gdbarch, arm_register_name);
> + set_tdesc_pseudo_register_name (gdbarch, arm_pseudo_register_name);
>
> tdesc_use_registers (gdbarch, tdesc, std::move (tdesc_data));
>
> @@ -9564,6 +9669,8 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
> (int) tdep->have_neon_pseudos);
> fprintf_unfiltered (file, _("arm_dump_tdep: have_neon = %i\n"),
> (int) tdep->have_neon);
> + fprintf_unfiltered (file, _("arm_dump_tdep: have_mve_pseudos = %i\n"),
> + (int) tdep->have_mve_pseudos);
> fprintf_unfiltered (file, _("arm_dump_tdep: Lowest pc = 0x%lx\n"),
> (unsigned long) tdep->lowest_pc);
> }
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 8bff27c940dec2c5a627459b343a33f3b4f886a1..8348565a6a0c2ba16049030e00e270d2c641ddaa 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -45329,6 +45329,9 @@ and @samp{xpsr}.
> The @samp{org.gnu.gdb.arm.fpa} feature is optional. If present, it
> should contain registers @samp{f0} through @samp{f7} and @samp{fps}.
>
> +The @samp{org.gnu.gdb.arm.m-profile-mve} feature is optional. If present, it
> +should contain register @samp{vpr}.
> +
Should we document what the individual fields in VPR mean?
> The @samp{org.gnu.gdb.xscale.iwmmxt} feature is optional. If present,
> it should contain at least registers @samp{wR0} through @samp{wR15} and
> @samp{wCGR0} through @samp{wCGR3}. The @samp{wCID}, @samp{wCon},
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index 689603847a001ae0297558cd1fde1242b1c9800c..51db08fe9fd2e8197613b971b8f9fbe2b8daf0f3 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -208,6 +208,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
> arm/arm-core.xml \
> arm/arm-fpa.xml \
> arm/arm-m-profile.xml \
> + arm/arm-m-profile-with-mve.xml \
> arm/arm-m-profile-with-fpa.xml \
> arm/arm-vfpv2.xml \
> arm/arm-vfpv3.xml \
> diff --git a/gdb/features/arm/arm-m-profile-with-mve.c b/gdb/features/arm/arm-m-profile-with-mve.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b23e77f7427b197a927d31cf1bc3a9f5a2354d4e
> --- /dev/null
> +++ b/gdb/features/arm/arm-m-profile-with-mve.c
> @@ -0,0 +1,21 @@
> +/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
> + Original: arm-m-profile-with-mve.xml */
> +
> +#include "gdbsupport/tdesc.h"
> +
> +static int
> +create_feature_arm_arm_m_profile_with_mve (struct target_desc *result, long regnum)
> +{
> + struct tdesc_feature *feature;
> +
> + feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile-mve");
> + tdesc_type_with_fields *type_with_fields;
> + type_with_fields = tdesc_create_flags (feature, "vpr_reg", 4);
> + tdesc_add_bitfield (type_with_fields, "P0", 0, 15);
> + tdesc_add_bitfield (type_with_fields, "MASK01", 16, 19);
> + tdesc_add_bitfield (type_with_fields, "MASK23", 20, 23);
> + tdesc_add_bitfield (type_with_fields, "RES0", 24, 31);
> +
> + tdesc_create_reg (feature, "vpr", regnum++, 1, NULL, 32, "vpr_reg");
> + return regnum;
> +}
> diff --git a/gdb/features/arm/arm-m-profile-with-mve.xml b/gdb/features/arm/arm-m-profile-with-mve.xml
> new file mode 100644
> index 0000000000000000000000000000000000000000..35b17351e0ca0c418c138c3eb711b4e54daac95c
> --- /dev/null
> +++ b/gdb/features/arm/arm-m-profile-with-mve.xml
> @@ -0,0 +1,21 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2021 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.arm.m-profile-mve">
> + <flags id="vpr_reg" size="4">
> + <!-- ARMv8.1-M and MVE: Unprivileged and privileged Access. -->
> + <field name="P0" start="0" end="15"/>
> + <!-- ARMv8.1-M: Privileged Access only. -->
> + <field name="MASK01" start="16" end="19"/>
> + <!-- ARMv8.1-M: Privileged Access only. -->
> + <field name="MASK23" start="20" end="23"/>
> + <!-- ARMv8.1-M: Privileged Access only. -->
> + <field name="RES0" start="24" end="31"/>
> + </flags>
> + <reg name="vpr" bitsize="32" type="vpr_reg"/>
> +</feature>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [GDB][PATCH] arm: Add support for mve vector predicate status and control register (vpr) and pseudo register p0.
2021-07-05 10:39 ` Luis Machado
@ 2021-07-05 11:03 ` Peter Maydell
0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2021-07-05 11:03 UTC (permalink / raw)
To: Luis Machado; +Cc: Srinath Parvathaneni, gdb-patches, Alan Hayward
On Mon, 5 Jul 2021 at 11:39, Luis Machado <luis.machado@linaro.org> wrote:
>
> Hi Srinath,
>
>
> On 7/2/21 5:47 AM, Srinath Parvathaneni via Gdb-patches wrote:
> > Hello,
> >
> > This patch add support for mve vector predication status and control register (vpr) and pseudo register p0.
> > Pseudo register p0 is the least significant bits of vpr and can be accessed as $p0, $vpr.p0.
> > For more information about the register please refer to [1].
> >
> > [1] https://developer.arm.com/documentation/ddi0553/bn
> > @@ -83,6 +84,7 @@ enum arm_m_profile_type {
> > ARM_M_TYPE_M_PROFILE,
> > ARM_M_TYPE_VFP_D16,
> > ARM_M_TYPE_WITH_FPA,
> > + ARM_M_TYPE_MVE,
> > ARM_M_TYPE_INVALID
> > };
Not in this patch, but since I see it going by, what is "M_TYPE_WITH_FPA" ?
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/features/arm/arm-m-profile-with-fpa.xml;h=fceeaea71771f12754bb19640aa951ddd8e18452;hb=HEAD
seems to think it is a combination of an M-profile CPU with the ancient FPA
floating point unit registers (for the hardware that preceded VFP). This
doesn't correspond to any actual hardware, because M-profile FP is always VFP.
> > @@ -9270,7 +9368,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > does not support that. */
> > feature = tdesc_find_feature (tdesc,
> > "org.gnu.gdb.arm.neon");
> > - if (feature != NULL)
> > + /* If we have VFP and no NEON, check for MVE. MVE uses NEON pseudos
> > + q0 to q7. */
Why "if we have VFP and no NEON" ? Integer-only MVE is a permitted
implementation,
in which case there is no requirement to implement the "FP extension".
> > + if (feature != NULL || is_mve)
> > {
> > /* NEON requires 32 double-precision registers. */
> > if (i != 32)
This change tries to use this code path for MVE, but this condition here reads
/* NEON requires 32 double-precision registers. */
if (i != 32)
return NULL;
which will always fail for MVE, because MVE has only registers
D0..D15, not D16..D31.
It looks like quite a lot of the code assumes that have_neon_pseudos
implies "have Q0..Q15",
so setting it to true for the MVE case will have knock-on effects
elsewhere anyway.
> > +static int
> > +create_feature_arm_arm_m_profile_with_mve (struct target_desc *result, long regnum)
> > +{
> > + struct tdesc_feature *feature;
> > +
> > + feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile-mve");
> > + tdesc_type_with_fields *type_with_fields;
> > + type_with_fields = tdesc_create_flags (feature, "vpr_reg", 4);
> > + tdesc_add_bitfield (type_with_fields, "P0", 0, 15);
> > + tdesc_add_bitfield (type_with_fields, "MASK01", 16, 19);
> > + tdesc_add_bitfield (type_with_fields, "MASK23", 20, 23);
> > + tdesc_add_bitfield (type_with_fields, "RES0", 24, 31);
Is it usual in GDB to define field masks for RES0/RES1 fields of registers?
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-05 11:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02 8:47 [GDB][PATCH] arm: Add support for mve vector predicate status and control register (vpr) and pseudo register p0 Srinath Parvathaneni
2021-07-02 15:44 ` Alan Hayward
2021-07-05 10:39 ` Luis Machado
2021-07-05 11:03 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).