gdb/riscv: Take CSR names from target description

First, consider the RISC-V register $x1.  This register has an alias
$ra.  When GDB processes an incoming target description we allow the
target to use either register name to describe the target.

However, within GDB's UI we want to use the $ra alias in preference to
the $x1 architecture name.

To achieve this GDB overrides the tdesc_register_name callback with
riscv_register_name.  In riscv_register_name we ensure that we always
return the preferred name, so in this case "ra".

To ensure the user can still access the register as $x1 if they want
to, when in riscv_check_tdesc_feature we spot that the target has
supplied the register, we add aliases for every name except the
preferred one, so in this case we add the alias "x1".

This scheme seems to work quite well, the targets have the flexibility
to be architecture focused if they wish (using x0 - x31) while GDB is
still using the ABI names ra, sp, gp, etc.

When this code was originally added there was an attempt made to
include the CSRs in the same scheme.  At the time the CSRs only had
two names, one pulled from riscv-opc.h, and one generated in GDB that
had the pattern csr%d.

The idea was that if the remote targets description described the CSRs
as csr%d then GDB would rename these back to the real CSR name.  This
code was only included because if followed the same pattern as the
x-regs and f-regs, not because I was actually aware of any target that
did this.

However, recent changes to add additional CSR aliases has made me
rethink the position here.

Lets consider the CSR $dscratch0.  This register has an alias
'csr1970' (1970 is 0x7b2, which is the offset of the CSR register into
the CSR address space).  However, this register was originally called
just 'dscratch', and so, after recent commits, this register also has
the alias 'dscratch'.

As the riscv-opc.h file calls this register 'dscratch0' GDB's
preferred name for this register is 'dscratch0'.

So, if the remote target description includes the register
'dscratch0', then GDB will add the aliases 'dscratch', and 'csr1970'.
In the UI GDB will describe the register as 'dscratch0', and all it
good.

The problem I see in this case is where the target describes the
register as 'dscratch'.  In this case GDB will still spot the register
and add the aliases 'dscratch', and 'csr1970', GDB will then give the
register the preferred name 'dscratch0'.

I don't like this.  For the CSRs I think that we should stick with the
naming scheme offered by the remote target description.  As the RISC-V
specification evolves and CSR register names evolve, insisting on
referring to registers by the most up to date name makes it harder for
a target to provide a consistent target description for an older
version of the RISC-V architecture spec.

In this precise case the target offers 'dscratch', which is from an
older version of the RISC-V specification, the newer version of the
spec has two registers 'dscratch0' and 'dscratch1'.  If we insist on
using 'dscratch0' it is then a little "weird" (or seems so to me) when
'dscratch1' is missing.

This patch makes a distinction between the x and f registers and the
other register sets.  For x and f we still make use of the renaming
scheme, forcing GDB to prefer the ABI name.  But after this patch the
CSR register group, and also the virtual register group, will always
prefer to use the name given in the target description, adding other
names as aliases, but not making any other name the preferred name.

gdb/ChangeLog:

	* riscv-tdep.c (struct riscv_register_feature::register_info): Fix
	whitespace error for declaration of names member variable.
	(struct riscv_register_feature): Add new prefer_first_name member
	variable, and fix whitespace error in declaration of registers.
	(riscv_xreg_feature): Initialize prefer_first_name field.
	(riscv_freg_feature): Likewise.
	(riscv_virtual_feature): Likewise.
	(riscv_csr_feature): Likewise.
	(riscv_register_name): Expand on comments.  Remove register name
	modifications for CSR and virtual registers.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-tdesc-regs.exp: Extend test case.

1 files changed, 38 insertions, 1 deletions

diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
index 46e64d6..63ac8fb 100644
--- a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # Various tests to check which register names are available after
-# loading a new target description file.
+# loading a new target description file, and which registers show up
+# in the output of the 'info registers' command.
 
 if {![istarget "riscv*-*-*"]} {
     verbose "Skipping ${gdb_test_file_name}."
@@ -79,3 +80,39 @@ gdb_test "info registers \$csr0" "Invalid register `csr0'"
 gdb_test "info registers \$dscratch0" "dscratch0\[ \t\]+.*"
 gdb_test "info registers \$dscratch" "dscratch\[ \t\]+.*"
 
+foreach rgroup {all save restore} {
+    # Now use 'info registers all' to see how many times the floating
+    # point status registers show up in the output.
+    array set reg_counts {}
+    set test "info registers $rgroup"
+    gdb_test_multiple $test $test {
+	-re ".*info registers all\r\n" {
+	    verbose -log "Skip to first register"
+	    exp_continue
+	}
+	-re "^(\[^ \t\]+)\[ \t\]+\[^\r\n\]+\r\n" {
+	    set reg $expect_out(1,string)
+	    incr reg_counts($reg)
+	exp_continue
+	}
+	-re "^$gdb_prompt $" {
+	    # Done.
+	}
+    }
+
+    foreach reg {dscratch} {
+	if { [info exists reg_counts($reg) ] } {
+	    set count $reg_counts($reg)
+	} else {
+	    set count 0
+	}
+	if {$reg == "dscratch" && $rgroup != "all"} {
+	    gdb_assert {$count == 0} \
+		"register $reg not seen in reggroup $rgroup"
+	} else {
+	    gdb_assert {$count == 1} \
+		"register $reg seen once in reggroup $rgroup"
+	}
+    }
+    array unset reg_counts
+}