* [PATCH] gdb/riscv: add support for vector registers in target descriptions @ 2021-05-05 9:46 Andrew Burgess 2021-05-05 21:05 ` Jim Wilson 2021-06-21 11:04 ` [PATCHv2] " Andrew Burgess 0 siblings, 2 replies; 5+ messages in thread From: Andrew Burgess @ 2021-05-05 9:46 UTC (permalink / raw) To: gdb-patches This commit adds support to RISC-V GDB for vector registers in the incoming target description. The vector registers should be described in a feature called "org.gnu.gdb.riscv.vector", and should contain the register v0 to v31. There's no restriction on the size or type of these registers, so the target description can set these up as it requires. However, if the target feature is present then all of the registers must be present, and they must all be the same size, these requirements are, I believe, inline with the RISC-V vector extension. The DWARF register numbers for the vector registers have been added, and the code to map between GDB's internal numbering and the DWARF numbering has been updated. I have not yet added a feature/riscv/*.xml file for the vector extension, the consequence of this is that we can't, right now, detect vector registers on a native target, this patch is all about supporting vectors on a remote target. It is worth noting that I don't actually have access to a RISC-V target with vectors, so the only testing that this patch has had has been done using 'set tdesc filename ....' to load a target description to which I have manually added the vector feature. This has shown that the vector register feature can be successfully parsed, and that the registers show up in the expected register groups. gdb/ChangeLog: * arch/riscv.c (riscv_create_target_description): GDB doesn't currently create target descriptions containing vector registers. * arch/riscv.h (struct riscv_gdbarch_features) <vlen>: New member variable. <operator==>: Also compare vlen. <hash>: Also include vlen. * riscv-tdep.c (riscv_feature_name_vector): New static global. (struct riscv_vector_feature): New struct. (riscv_vector_feature): New static global. (riscv_register_reggroup_p): Ensure vector registers are part of the 'all' group, and part of the 'vector' group. (riscv_dwarf_reg_to_regnum): Handle vector registers. (riscv_gdbarch_init): Check vector register feature. * riscv-tdep.h: Add vector registers to GDB's internal register numbers, and to the DWARF register numbers. --- gdb/ChangeLog | 18 +++++++ gdb/arch/riscv.c | 6 +++ gdb/arch/riscv.h | 12 ++++- gdb/riscv-tdep.c | 121 +++++++++++++++++++++++++++++++++++++++++++++-- gdb/riscv-tdep.h | 8 +++- 5 files changed, 159 insertions(+), 6 deletions(-) diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c index 8fbcad1a8ea..85d60a3e257 100644 --- a/gdb/arch/riscv.c +++ b/gdb/arch/riscv.c @@ -84,6 +84,12 @@ riscv_create_target_description (const struct riscv_gdbarch_features features) else if (features.flen == 8) regnum = create_feature_riscv_64bit_fpu (tdesc.get (), regnum); + /* Currently GDB only supports vector features coming from remote + targets. We don't support creating vector features on native targets + (yet). */ + if (features.vlen != 0) + error (_("unable to create vector feature")); + return tdesc; } diff --git a/gdb/arch/riscv.h b/gdb/arch/riscv.h index faa038a713e..65a998bb2b9 100644 --- a/gdb/arch/riscv.h +++ b/gdb/arch/riscv.h @@ -46,13 +46,20 @@ struct riscv_gdbarch_features that there are no f-registers. No other value is valid. */ int flen = 0; + /* The size of the v-registers in bytes. The value 0 indicates a target + with no vector registers. The minimum value for a standard compliant + target should be 16, but GDB doesn't currently mind, and will accept + any vector size. */ + int vlen = 0; + /* When true this target is RV32E. */ bool embedded = false; /* Equality operator. */ bool operator== (const struct riscv_gdbarch_features &rhs) const { - return (xlen == rhs.xlen && flen == rhs.flen && embedded == rhs.embedded); + return (xlen == rhs.xlen && flen == rhs.flen + && embedded == rhs.embedded && vlen == rhs.vlen); } /* Inequality operator. */ @@ -66,7 +73,8 @@ struct riscv_gdbarch_features { std::size_t val = ((embedded ? 1 : 0) << 10 | (xlen & 0x1f) << 5 - | (flen & 0x1f) << 0); + | (flen & 0x1f) << 0 + | (vlen & 0xfff) << 11); return val; } }; diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index c1783996846..cbcaa7c9155 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -99,6 +99,7 @@ const char *riscv_feature_name_csr = "org.gnu.gdb.riscv.csr"; static const char *riscv_feature_name_cpu = "org.gnu.gdb.riscv.cpu"; static const char *riscv_feature_name_fpu = "org.gnu.gdb.riscv.fpu"; static const char *riscv_feature_name_virtual = "org.gnu.gdb.riscv.virtual"; +static const char *riscv_feature_name_vector = "org.gnu.gdb.riscv.vector"; /* Cached information about a frame. */ @@ -575,6 +576,115 @@ struct riscv_csr_feature : public riscv_register_feature static const struct riscv_csr_feature riscv_csr_feature; +/* Class representing the f-registers feature set. */ + +struct riscv_vector_feature : public riscv_register_feature +{ + riscv_vector_feature () + : riscv_register_feature (riscv_feature_name_vector) + { + m_registers = { + { RISCV_V0_REGNUM + 0, { "v0" } }, + { RISCV_V0_REGNUM + 1, { "v1" } }, + { RISCV_V0_REGNUM + 2, { "v2" } }, + { RISCV_V0_REGNUM + 3, { "v3" } }, + { RISCV_V0_REGNUM + 4, { "v4" } }, + { RISCV_V0_REGNUM + 5, { "v5" } }, + { RISCV_V0_REGNUM + 6, { "v6" } }, + { RISCV_V0_REGNUM + 7, { "v7" } }, + { RISCV_V0_REGNUM + 8, { "v8" } }, + { RISCV_V0_REGNUM + 9, { "v9" } }, + { RISCV_V0_REGNUM + 10, { "v10" } }, + { RISCV_V0_REGNUM + 11, { "v11" } }, + { RISCV_V0_REGNUM + 12, { "v12" } }, + { RISCV_V0_REGNUM + 13, { "v13" } }, + { RISCV_V0_REGNUM + 14, { "v14" } }, + { RISCV_V0_REGNUM + 15, { "v15" } }, + { RISCV_V0_REGNUM + 16, { "v16" } }, + { RISCV_V0_REGNUM + 17, { "v17" } }, + { RISCV_V0_REGNUM + 18, { "v18" } }, + { RISCV_V0_REGNUM + 19, { "v19" } }, + { RISCV_V0_REGNUM + 20, { "v20" } }, + { RISCV_V0_REGNUM + 21, { "v21" } }, + { RISCV_V0_REGNUM + 22, { "v22" } }, + { RISCV_V0_REGNUM + 23, { "v23" } }, + { RISCV_V0_REGNUM + 24, { "v24" } }, + { RISCV_V0_REGNUM + 25, { "v25" } }, + { RISCV_V0_REGNUM + 26, { "v26" } }, + { RISCV_V0_REGNUM + 27, { "v27" } }, + { RISCV_V0_REGNUM + 28, { "v28" } }, + { RISCV_V0_REGNUM + 29, { "v29" } }, + { RISCV_V0_REGNUM + 30, { "v30" } }, + { RISCV_V0_REGNUM + 31, { "v31" } }, + }; + } + + /* Return the preferred name for the register with gdb register number + REGNUM, which must be in the inclusive range RISCV_V0_REGNUM to + RISCV_V0_REGNUM + 31. */ + const char *register_name (int regnum) const + { + gdb_assert (regnum >= RISCV_V0_REGNUM + && regnum <= RISCV_V0_REGNUM + 31); + regnum -= RISCV_V0_REGNUM; + return m_registers[regnum].names[0]; + } + + /* Check this feature within TDESC, record the registers from this + feature into TDESC_DATA and update ALIASES and FEATURES. */ + bool check (const struct target_desc *tdesc, + struct tdesc_arch_data *tdesc_data, + std::vector<riscv_pending_register_alias> *aliases, + struct riscv_gdbarch_features *features) const + { + const struct tdesc_feature *feature_vector = tdesc_feature (tdesc); + + /* It's fine if this feature is missing. Update the architecture + feature set and return. */ + if (feature_vector == nullptr) + { + features->vlen = 0; + return true; + } + + /* Check all of the vector registers are present. */ + for (const auto ® : m_registers) + { + if (!reg.check (tdesc_data, feature_vector, true, aliases)) + return false; + } + + /* Look through all of the vector registers and check they all have the + same bitsize. Use this bitsize to update the feature set for this + gdbarch. */ + int vector_bitsize = -1; + for (const auto ® : m_registers) + { + int reg_bitsize = -1; + for (const char *name : reg.names) + { + if (tdesc_unnumbered_register (feature_vector, name)) + { + reg_bitsize = tdesc_register_bitsize (feature_vector, name); + break; + } + } + gdb_assert (reg_bitsize != -1); + if (vector_bitsize == -1) + vector_bitsize = reg_bitsize; + else if (vector_bitsize != reg_bitsize) + return false; + } + + features->vlen = (vector_bitsize / 8); + return true; + } +}; + +/* An instance of the f-register feature set. */ + +static const struct riscv_vector_feature riscv_vector_feature; + /* Controls whether we place compressed breakpoints or not. When in auto mode GDB tries to determine if the target supports compressed breakpoints, and uses them if it does. */ @@ -1192,7 +1302,7 @@ riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum, if (reggroup == all_reggroup) { - if (regnum < RISCV_FIRST_CSR_REGNUM || regnum == RISCV_PRIV_REGNUM) + if (regnum < RISCV_FIRST_CSR_REGNUM || regnum >= RISCV_PRIV_REGNUM) return 1; if (riscv_is_regnum_a_named_csr (regnum)) return 1; @@ -1226,7 +1336,7 @@ riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum, return 0; } else if (reggroup == vector_reggroup) - return 0; + return (regnum >= RISCV_V0_REGNUM && regnum <= RISCV_V31_REGNUM); else return 0; } @@ -3320,6 +3430,9 @@ riscv_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg) else if (reg >= RISCV_DWARF_FIRST_CSR && reg <= RISCV_DWARF_LAST_CSR) return RISCV_FIRST_CSR_REGNUM + (reg - RISCV_DWARF_FIRST_CSR); + else if (reg >= RISCV_DWARF_REGNUM_V0 && reg <= RISCV_DWARF_REGNUM_V31) + return RISCV_V0_REGNUM + (reg - RISCV_DWARF_REGNUM_V0); + return -1; } @@ -3488,7 +3601,9 @@ riscv_gdbarch_init (struct gdbarch_info info, && riscv_virtual_feature.check (tdesc, tdesc_data.get (), &pending_aliases, &features) && riscv_csr_feature.check (tdesc, tdesc_data.get (), - &pending_aliases, &features)); + &pending_aliases, &features) + && riscv_vector_feature.check (tdesc, tdesc_data.get (), + &pending_aliases, &features)); if (!valid_p) { if (riscv_debug_gdbarch) diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h index 154097df5d0..62bf4797d02 100644 --- a/gdb/riscv-tdep.h +++ b/gdb/riscv-tdep.h @@ -53,7 +53,11 @@ enum RISCV_PRIV_REGNUM = 4161, - RISCV_LAST_REGNUM = RISCV_PRIV_REGNUM + RISCV_V0_REGNUM, + + RISCV_V31_REGNUM = RISCV_V0_REGNUM + 31, + + RISCV_LAST_REGNUM = RISCV_V31_REGNUM }; /* RiscV DWARF register numbers. */ @@ -63,6 +67,8 @@ enum RISCV_DWARF_REGNUM_X31 = 31, RISCV_DWARF_REGNUM_F0 = 32, RISCV_DWARF_REGNUM_F31 = 63, + RISCV_DWARF_REGNUM_V0 = 96, + RISCV_DWARF_REGNUM_V31 = 127, RISCV_DWARF_FIRST_CSR = 4096, RISCV_DWARF_LAST_CSR = 8191, }; -- 2.25.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb/riscv: add support for vector registers in target descriptions 2021-05-05 9:46 [PATCH] gdb/riscv: add support for vector registers in target descriptions Andrew Burgess @ 2021-05-05 21:05 ` Jim Wilson 2021-05-06 7:26 ` Palmer Dabbelt 2021-06-21 11:04 ` [PATCHv2] " Andrew Burgess 1 sibling, 1 reply; 5+ messages in thread From: Jim Wilson @ 2021-05-05 21:05 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches On Wed, May 5, 2021 at 2:46 AM Andrew Burgess <andrew.burgess@embecosm.com> wrote: > This commit adds support to RISC-V GDB for vector registers in the > incoming target description. > It looks reasonable to me. Though I did notice that you have two comments which mention f-register instead of v-register. The vector spec isn't official yet, and hasn't been frozen yet, so there is a chance that some things will still change, but very unlikely that this patch would be affected. I can't imagine them changing the number of user registers at this point. Jim ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb/riscv: add support for vector registers in target descriptions 2021-05-05 21:05 ` Jim Wilson @ 2021-05-06 7:26 ` Palmer Dabbelt 0 siblings, 0 replies; 5+ messages in thread From: Palmer Dabbelt @ 2021-05-06 7:26 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches On Wed, 05 May 2021 14:05:52 PDT (-0700), Jim Wilson wrote: > On Wed, May 5, 2021 at 2:46 AM Andrew Burgess <andrew.burgess@embecosm.com> > wrote: > >> This commit adds support to RISC-V GDB for vector registers in the >> incoming target description. >> > > It looks reasonable to me. Though I did notice that you have two comments > which mention f-register instead of v-register. > > The vector spec isn't official yet, and hasn't been frozen yet, so there is > a chance that some things will still change, but very unlikely that this > patch would be affected. I can't imagine them changing the number of user > registers at this point. IMO that's fine: we can maintain a stable ABI for this, even in the unlikely event that the number of vector registers changes we'll just end up with some unused register indices. We're already relying on an external mechanism to determine the semantics of the CSRs, as we've allocated register numbers for all of those despite most not being in any spec. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCHv2] gdb/riscv: add support for vector registers in target descriptions 2021-05-05 9:46 [PATCH] gdb/riscv: add support for vector registers in target descriptions Andrew Burgess 2021-05-05 21:05 ` Jim Wilson @ 2021-06-21 11:04 ` Andrew Burgess 2021-06-21 11:41 ` Eli Zaretskii via Gdb-patches 1 sibling, 1 reply; 5+ messages in thread From: Andrew Burgess @ 2021-06-21 11:04 UTC (permalink / raw) To: gdb-patches Thanks for feedback on v1. I agree that there is a very small risk that the vector register set could change for the final version of the spec, but like you both seem to say, this seems unlikely. And even if it did change I don't think changing the requirements of the feature for a later release of GDB would be that bad. So, I'm proposing that once this has had a doc review, I'll get this merged, hopefully in time for the next release (of GDB). In v2: - Fixed the f-register/v-register typo that Jim pointed out, - Added documentation of the new feature to the manual, - Added a NEWS entry, - Updated the commit message to mention that the vector extension is still in draft. Thanks, Andrew --- commit 2a909530fd8b98ee31fb59ba9019a641029cff35 Author: Andrew Burgess <andrew.burgess@embecosm.com> Date: Tue May 4 11:41:09 2021 +0100 gdb/riscv: add support for vector registers in target descriptions This commit adds support to RISC-V GDB for vector registers in the incoming target description. The vector registers should be described in a feature called "org.gnu.gdb.riscv.vector", and should contain the register v0 to v31. There's no restriction on the size or type of these registers, so the target description can set these up as it requires. However, if the target feature is present then all of the registers must be present, and they must all be the same size, these requirements are, I believe, inline with the RISC-V vector extension. The DWARF register numbers for the vector registers have been added, and the code to map between GDB's internal numbering and the DWARF numbering has been updated. I have not yet added a feature/riscv/*.xml file for the vector extension, the consequence of this is that we can't, right now, detect vector registers on a native target, this patch is all about supporting vectors on a remote target. It is worth noting that I don't actually have access to a RISC-V target with vectors, so the only testing that this patch has had has been done using 'set tdesc filename ....' to load a target description to which I have manually added the vector feature. This has shown that the vector register feature can be successfully parsed, and that the registers show up in the expected register groups. Additionally, the RISC-V vector extension is currently at v0.10, which is also the v1.0 draft release. However, this extension is not yet finalised. It is possible (but unlikely I think) that the register set could change between now and the final release of the vector extension. If this were to happen then we would potentially end up changing the requirements for the new org.gnu.gdb.riscv.vector feature. I really don't think it is likely that the register set will change this late in the process, and even if it did, changing the feature requirements will not be a problem as far as I am concerned (when the alternative is GDB just continues without this feature for now). gdb/ChangeLog: * NEWS: Mention new target feature name. * arch/riscv.c (riscv_create_target_description): GDB doesn't currently create target descriptions containing vector registers. * arch/riscv.h (struct riscv_gdbarch_features) <vlen>: New member variable. <operator==>: Also compare vlen. <hash>: Also include vlen. * riscv-tdep.c (riscv_feature_name_vector): New static global. (struct riscv_vector_feature): New struct. (riscv_vector_feature): New static global. (riscv_register_reggroup_p): Ensure vector registers are part of the 'all' group, and part of the 'vector' group. (riscv_dwarf_reg_to_regnum): Handle vector registers. (riscv_gdbarch_init): Check vector register feature. * riscv-tdep.h: Add vector registers to GDB's internal register numbers, and to the DWARF register numbers. gdb/doc/ChangeLog: * gdb.texinfo (RISC-V Features): Mention vector register feature. diff --git a/gdb/NEWS b/gdb/NEWS index 56743fc9aea..6bf5e44da99 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -78,6 +78,11 @@ and "-eiex" that allow options (that would normally appear in a gdbearlyinit file) to be passed on the command line. +* For RISC-V targets, the target feature "org.gnu.gdb.riscv.vector" is + now understood by GDB, and can be used to describe the vector + registers of a target. The precise requirements of this register + feature are documented in the GDB manual. + * New commands set debug event-loop diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c index 8fbcad1a8ea..85d60a3e257 100644 --- a/gdb/arch/riscv.c +++ b/gdb/arch/riscv.c @@ -84,6 +84,12 @@ riscv_create_target_description (const struct riscv_gdbarch_features features) else if (features.flen == 8) regnum = create_feature_riscv_64bit_fpu (tdesc.get (), regnum); + /* Currently GDB only supports vector features coming from remote + targets. We don't support creating vector features on native targets + (yet). */ + if (features.vlen != 0) + error (_("unable to create vector feature")); + return tdesc; } diff --git a/gdb/arch/riscv.h b/gdb/arch/riscv.h index faa038a713e..65a998bb2b9 100644 --- a/gdb/arch/riscv.h +++ b/gdb/arch/riscv.h @@ -46,13 +46,20 @@ struct riscv_gdbarch_features that there are no f-registers. No other value is valid. */ int flen = 0; + /* The size of the v-registers in bytes. The value 0 indicates a target + with no vector registers. The minimum value for a standard compliant + target should be 16, but GDB doesn't currently mind, and will accept + any vector size. */ + int vlen = 0; + /* When true this target is RV32E. */ bool embedded = false; /* Equality operator. */ bool operator== (const struct riscv_gdbarch_features &rhs) const { - return (xlen == rhs.xlen && flen == rhs.flen && embedded == rhs.embedded); + return (xlen == rhs.xlen && flen == rhs.flen + && embedded == rhs.embedded && vlen == rhs.vlen); } /* Inequality operator. */ @@ -66,7 +73,8 @@ struct riscv_gdbarch_features { std::size_t val = ((embedded ? 1 : 0) << 10 | (xlen & 0x1f) << 5 - | (flen & 0x1f) << 0); + | (flen & 0x1f) << 0 + | (vlen & 0xfff) << 11); return val; } }; diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 7cdc83b9537..dfc00b120b4 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -46349,6 +46349,14 @@ feature if the target has the floating point control registers, but no other floating point hardware. +The @samp{org.gnu.gdb.riscv.vector} feature is optional. If present, +it should contain registers @samp{v0} through @samp{v31}, all of which +must be the same size. These requirements are based on the v0.10 +draft vector extension, as the vector extension is not yet final. In +the event that the register set of the vector extension changes for +the final specification, the requirements given here could change for +future releases of @value{GDBN}. + @node RX Features @subsection RX Features @cindex target descriptions, RX Features diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 7fae9140155..d942b059f55 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -99,6 +99,7 @@ const char *riscv_feature_name_csr = "org.gnu.gdb.riscv.csr"; static const char *riscv_feature_name_cpu = "org.gnu.gdb.riscv.cpu"; static const char *riscv_feature_name_fpu = "org.gnu.gdb.riscv.fpu"; static const char *riscv_feature_name_virtual = "org.gnu.gdb.riscv.virtual"; +static const char *riscv_feature_name_vector = "org.gnu.gdb.riscv.vector"; /* Cached information about a frame. */ @@ -575,6 +576,115 @@ struct riscv_csr_feature : public riscv_register_feature static const struct riscv_csr_feature riscv_csr_feature; +/* Class representing the v-registers feature set. */ + +struct riscv_vector_feature : public riscv_register_feature +{ + riscv_vector_feature () + : riscv_register_feature (riscv_feature_name_vector) + { + m_registers = { + { RISCV_V0_REGNUM + 0, { "v0" } }, + { RISCV_V0_REGNUM + 1, { "v1" } }, + { RISCV_V0_REGNUM + 2, { "v2" } }, + { RISCV_V0_REGNUM + 3, { "v3" } }, + { RISCV_V0_REGNUM + 4, { "v4" } }, + { RISCV_V0_REGNUM + 5, { "v5" } }, + { RISCV_V0_REGNUM + 6, { "v6" } }, + { RISCV_V0_REGNUM + 7, { "v7" } }, + { RISCV_V0_REGNUM + 8, { "v8" } }, + { RISCV_V0_REGNUM + 9, { "v9" } }, + { RISCV_V0_REGNUM + 10, { "v10" } }, + { RISCV_V0_REGNUM + 11, { "v11" } }, + { RISCV_V0_REGNUM + 12, { "v12" } }, + { RISCV_V0_REGNUM + 13, { "v13" } }, + { RISCV_V0_REGNUM + 14, { "v14" } }, + { RISCV_V0_REGNUM + 15, { "v15" } }, + { RISCV_V0_REGNUM + 16, { "v16" } }, + { RISCV_V0_REGNUM + 17, { "v17" } }, + { RISCV_V0_REGNUM + 18, { "v18" } }, + { RISCV_V0_REGNUM + 19, { "v19" } }, + { RISCV_V0_REGNUM + 20, { "v20" } }, + { RISCV_V0_REGNUM + 21, { "v21" } }, + { RISCV_V0_REGNUM + 22, { "v22" } }, + { RISCV_V0_REGNUM + 23, { "v23" } }, + { RISCV_V0_REGNUM + 24, { "v24" } }, + { RISCV_V0_REGNUM + 25, { "v25" } }, + { RISCV_V0_REGNUM + 26, { "v26" } }, + { RISCV_V0_REGNUM + 27, { "v27" } }, + { RISCV_V0_REGNUM + 28, { "v28" } }, + { RISCV_V0_REGNUM + 29, { "v29" } }, + { RISCV_V0_REGNUM + 30, { "v30" } }, + { RISCV_V0_REGNUM + 31, { "v31" } }, + }; + } + + /* Return the preferred name for the register with gdb register number + REGNUM, which must be in the inclusive range RISCV_V0_REGNUM to + RISCV_V0_REGNUM + 31. */ + const char *register_name (int regnum) const + { + gdb_assert (regnum >= RISCV_V0_REGNUM + && regnum <= RISCV_V0_REGNUM + 31); + regnum -= RISCV_V0_REGNUM; + return m_registers[regnum].names[0]; + } + + /* Check this feature within TDESC, record the registers from this + feature into TDESC_DATA and update ALIASES and FEATURES. */ + bool check (const struct target_desc *tdesc, + struct tdesc_arch_data *tdesc_data, + std::vector<riscv_pending_register_alias> *aliases, + struct riscv_gdbarch_features *features) const + { + const struct tdesc_feature *feature_vector = tdesc_feature (tdesc); + + /* It's fine if this feature is missing. Update the architecture + feature set and return. */ + if (feature_vector == nullptr) + { + features->vlen = 0; + return true; + } + + /* Check all of the vector registers are present. */ + for (const auto ® : m_registers) + { + if (!reg.check (tdesc_data, feature_vector, true, aliases)) + return false; + } + + /* Look through all of the vector registers and check they all have the + same bitsize. Use this bitsize to update the feature set for this + gdbarch. */ + int vector_bitsize = -1; + for (const auto ® : m_registers) + { + int reg_bitsize = -1; + for (const char *name : reg.names) + { + if (tdesc_unnumbered_register (feature_vector, name)) + { + reg_bitsize = tdesc_register_bitsize (feature_vector, name); + break; + } + } + gdb_assert (reg_bitsize != -1); + if (vector_bitsize == -1) + vector_bitsize = reg_bitsize; + else if (vector_bitsize != reg_bitsize) + return false; + } + + features->vlen = (vector_bitsize / 8); + return true; + } +}; + +/* An instance of the v-register feature set. */ + +static const struct riscv_vector_feature riscv_vector_feature; + /* Controls whether we place compressed breakpoints or not. When in auto mode GDB tries to determine if the target supports compressed breakpoints, and uses them if it does. */ @@ -1192,7 +1302,7 @@ riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum, if (reggroup == all_reggroup) { - if (regnum < RISCV_FIRST_CSR_REGNUM || regnum == RISCV_PRIV_REGNUM) + if (regnum < RISCV_FIRST_CSR_REGNUM || regnum >= RISCV_PRIV_REGNUM) return 1; if (riscv_is_regnum_a_named_csr (regnum)) return 1; @@ -1226,7 +1336,7 @@ riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum, return 0; } else if (reggroup == vector_reggroup) - return 0; + return (regnum >= RISCV_V0_REGNUM && regnum <= RISCV_V31_REGNUM); else return 0; } @@ -3320,6 +3430,9 @@ riscv_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg) else if (reg >= RISCV_DWARF_FIRST_CSR && reg <= RISCV_DWARF_LAST_CSR) return RISCV_FIRST_CSR_REGNUM + (reg - RISCV_DWARF_FIRST_CSR); + else if (reg >= RISCV_DWARF_REGNUM_V0 && reg <= RISCV_DWARF_REGNUM_V31) + return RISCV_V0_REGNUM + (reg - RISCV_DWARF_REGNUM_V0); + return -1; } @@ -3488,7 +3601,9 @@ riscv_gdbarch_init (struct gdbarch_info info, && riscv_virtual_feature.check (tdesc, tdesc_data.get (), &pending_aliases, &features) && riscv_csr_feature.check (tdesc, tdesc_data.get (), - &pending_aliases, &features)); + &pending_aliases, &features) + && riscv_vector_feature.check (tdesc, tdesc_data.get (), + &pending_aliases, &features)); if (!valid_p) { if (riscv_debug_gdbarch) diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h index 154097df5d0..62bf4797d02 100644 --- a/gdb/riscv-tdep.h +++ b/gdb/riscv-tdep.h @@ -53,7 +53,11 @@ enum RISCV_PRIV_REGNUM = 4161, - RISCV_LAST_REGNUM = RISCV_PRIV_REGNUM + RISCV_V0_REGNUM, + + RISCV_V31_REGNUM = RISCV_V0_REGNUM + 31, + + RISCV_LAST_REGNUM = RISCV_V31_REGNUM }; /* RiscV DWARF register numbers. */ @@ -63,6 +67,8 @@ enum RISCV_DWARF_REGNUM_X31 = 31, RISCV_DWARF_REGNUM_F0 = 32, RISCV_DWARF_REGNUM_F31 = 63, + RISCV_DWARF_REGNUM_V0 = 96, + RISCV_DWARF_REGNUM_V31 = 127, RISCV_DWARF_FIRST_CSR = 4096, RISCV_DWARF_LAST_CSR = 8191, }; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv2] gdb/riscv: add support for vector registers in target descriptions 2021-06-21 11:04 ` [PATCHv2] " Andrew Burgess @ 2021-06-21 11:41 ` Eli Zaretskii via Gdb-patches 0 siblings, 0 replies; 5+ messages in thread From: Eli Zaretskii via Gdb-patches @ 2021-06-21 11:41 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches > Date: Mon, 21 Jun 2021 12:04:20 +0100 > From: Andrew Burgess <andrew.burgess@embecosm.com> > Cc: palmer@dabbelt.com, nelson.chu@sifive.com, jimw@sifive.com, > jhb@freebsd.org, Eli Zaretskii <eliz@gnu.org> > > Thanks for feedback on v1. > > I agree that there is a very small risk that the vector register set > could change for the final version of the spec, but like you both seem > to say, this seems unlikely. > > And even if it did change I don't think changing the requirements of > the feature for a later release of GDB would be that bad. > > So, I'm proposing that once this has had a doc review, I'll get this > merged, hopefully in time for the next release (of GDB). > > In v2: > > - Fixed the f-register/v-register typo that Jim pointed out, > - Added documentation of the new feature to the manual, > - Added a NEWS entry, > - Updated the commit message to mention that the vector extension is > still in draft. Thanks, the documentation parts are okay. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-21 11:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-05 9:46 [PATCH] gdb/riscv: add support for vector registers in target descriptions Andrew Burgess 2021-05-05 21:05 ` Jim Wilson 2021-05-06 7:26 ` Palmer Dabbelt 2021-06-21 11:04 ` [PATCHv2] " Andrew Burgess 2021-06-21 11:41 ` Eli Zaretskii via Gdb-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox