gdb/riscv: improve (and fix) display of frm field in 'info registers'

On RISC-V the FCSR (float control/status register) is split into two
parts, FFLAGS (the flags) and FRM (the rounding mode).  Both of these
two fields are part of the FCSR register, but can also be accessed as
separate registers in their own right.  And so, we have three separate
registers, $fflags, $frm, and $fcsr, with the last of these being the
combination of the first two.

Here's how the bits of FCSR are split between FRM and FFLAGS:

         ,--------- FFLAGS
       |---|
    76543210 <----- FCSR
    |-|
     '--------------FRM

Here's how GDB currently displays these registers:

  (gdb) info registers $fflags $frm $fcsr
  fflags         0x0      RD:0 NV:0 DZ:0 OF:0 UF:0 NX:0
  frm            0x0      FRM:0 [RNE (round to nearest; ties to even)]
  fcsr           0x0      RD:0 NV:0 DZ:0 OF:0 UF:0 NX:0 FRM:0 [RNE (round to nearest; ties to even)]

Notice the 'RD' field which is present in both $fflags and $fcsr.
This field contains the value of the FRM field, which makes sense when
displaying the $fcsr, but makes no sense when displaying $fflags, as
the $fflags doesn't include the FRM field.

Additionally, the $fcsr already includes an FRM field, so the
information in 'RD' is duplicated.  Consider this:

  (gdb) set $frm = 0x3
  (gdb) info registers $fflags $frm $fcsr                             │
  fflags         0x0      RD:0 NV:0 DZ:0 OF:0 UF:0 NX:0
  frm            0x3      FRM:3 [RUP (Round up towards +INF)]
  fcsr           0x60     RD:3 NV:0 DZ:0 OF:0 UF:0 NX:0 FRM:3 [RUP (Round up towards +INF)]

See how the 'RD' field in $fflags still displays 0, while the 'RD' and
'FRM' fields in $fcsr show the same information.

The first change I propose in this commit is to remove the 'RD'
field.  After this change the output now looks like this:

  (gdb) info registers $fflags $frm $fcsr
  fflags         0x0      NV:0 DZ:0 OF:0 UF:0 NX:0
  frm            0x0      FRM:0 [RNE (round to nearest; ties to even)]
  fcsr           0x0      NV:0 DZ:0 OF:0 UF:0 NX:0 FRM:0 [RNE (round to nearest; ties to even)]

Next, I spotted that the text that goes along with the 'FRM' field was
not wrapped in the i18n markers for internationalisation, so I added
those.

Next, I spotted that:

  (gdb) set $frm=0x7
  (gdb) info registers $fflags $frm $fcsr
  fflags         0x0      RD:0 NV:0 DZ:0 OF:0 UF:0 NX:0
  frm            0x7      FRM:3 [RUP (Round up towards +INF)]
  fcsr           0xe0     RD:7 NV:0 DZ:0 OF:0 UF:0 NX:0 FRM:3 [RUP (Round up towards +INF)]

Notice that despite being a 3-bit field, FRM masks to 2-bits.
Checking the manual I can see that the FRM field is 3-bits, and is
defined for all 8 values.  That GDB masks to 2-bits is just a bug I
think, so I've fixed this.

Finally, the 'FRM' text for value 0x7 is wrong.  Currently we use the
text 'dynamic rounding mode' for value 0x7.  However, this is not
really correct.

A RISC-V instruction can either encode the rounding mode within the
instruction, or a RISC-V instruction can choose to use a global,
dynamic rounding mode.

So, for the rounding-mode field of an _instruction_ the value 0x7
indicates "dynamic round mode", the instruction should defer to the
rounding mode held in the FRM field of the $fcsr.

But it makes no sense for the FRM of $fcsr to itself be set to
0x7 (dynamic rounding mode), and indeed, section 11.2, "Floating-Point
Control and Status Register" of the RISC-V manual, says that a value
of 0x7 in the $fcsr FRM field is invalid, and if an instruction has
_its_ round-mode set to dynamic, and the FRM field is also set to 0x7,
then an illegal instruction exception is raised.

And so, I propose changing the text for value 0x7 of the FRM field to
be "INVALID[7] (Dynamic rounding mode)".  We already use the text
"INVALID[5]" and "INVALID[6]" for the two other invalid fields,
however, I think adding the extra "Dynamic round mode" hint might be
helpful.

I've added a new test that uses 'info registers' to check what GDB
prints for the three registers related to this patch.  There is one
slight oddity with this test - for the fflags and frm registers, the
test accepts both the "normal" output (as described above), but also
allows these registers to be reported as '<unavailable>'.

The reason why I accept <unavailable> is that currently, the RISC-V,
native Linux target advertises these registers in its target
description, but then doesn't support reading or writing of these
registers, this results in the registers being reported as
unavailable.

A later patch in this series will address this issue, and will remove
this check for <unavailable>.

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 9ec430d8a10ca79561c5158f939d862c868da253..93ee597af58c189eeee5f64cc192c9e81596da5d 100644 (file)
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1172,8 +1172,7 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
              gdb_printf (file, "\t");
              if (regnum != RISCV_CSR_FRM_REGNUM)
                gdb_printf (file,
-                           "RD:%01X NV:%d DZ:%d OF:%d UF:%d NX:%d",
-                           (int) ((d >> 5) & 0x7),
+                           "NV:%d DZ:%d OF:%d UF:%d NX:%d",
                            (int) ((d >> 4) & 0x1),
                            (int) ((d >> 3) & 0x1),
                            (int) ((d >> 2) & 0x1),
@@ -1184,17 +1183,20 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
                {
                  static const char * const sfrm[] =
                    {
-                     "RNE (round to nearest; ties to even)",
-                     "RTZ (Round towards zero)",
-                     "RDN (Round down towards -INF)",
-                     "RUP (Round up towards +INF)",
-                     "RMM (Round to nearest; ties to max magnitude)",
-                     "INVALID[5]",
-                     "INVALID[6]",
-                     "dynamic rounding mode",
+                     _("RNE (round to nearest; ties to even)"),
+                     _("RTZ (Round towards zero)"),
+                     _("RDN (Round down towards -INF)"),
+                     _("RUP (Round up towards +INF)"),
+                     _("RMM (Round to nearest; ties to max magnitude)"),
+                     _("INVALID[5]"),
+                     _("INVALID[6]"),
+                     /* A value of 0x7 indicates dynamic rounding mode when
+                        used within an instructions rounding-mode field, but
+                        is invalid within the FRM register.  */
+                     _("INVALID[7] (Dynamic rounding mode)"),
                    };
                  int frm = ((regnum == RISCV_CSR_FCSR_REGNUM)
-                            ? (d >> 5) : d) & 0x3;
+                            ? (d >> 5) : d) & 0x7;
 
                  gdb_printf (file, "%sFRM:%i [%s]",
                              (regnum == RISCV_CSR_FCSR_REGNUM

diff --git a/gdb/testsuite/gdb.arch/riscv-info-fcsr.c b/gdb/testsuite/gdb.arch/riscv-info-fcsr.c
new file mode 100644 (file)
index 0000000..034c190
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-info-fcsr.c
@@ -0,0 +1,22 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  return 0;
+}

diff --git a/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp b/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
new file mode 100644 (file)
index 0000000..3d6095d
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
@@ -0,0 +1,147 @@
+# Copyright 2022 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check the formatting of the fcsr, fflags, and frm registers in the
+# output of the 'info registers' command.
+
+if {![istarget "riscv*-*-*"]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+if { [gdb_skip_float_test] } {
+    untested "no floating point support"
+    return
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+   return 0
+}
+
+# Merge FFLAGS_VALUE and FRM_VALUE into a single hexadecimal value
+# that can be written to the fcsr register.  The two arguments should
+# be the value of each of the two fields within the fcsr register.
+proc merge_fflags_and_frm { fflags_value frm_value } {
+    set fcsr_value 0x[format %x [expr $fflags_value | ($frm_value << 5)]]
+    return $fcsr_value
+}
+
+# Use 'info registers' to check the current values of the fflags, frm,
+# and fcsr registers.  The value in fcsr should consist of the
+# FFLAGS_VALUE and FRM_VALUE, and the frm field of the fcsr register
+# should have the text FRM_STRING associated with it.
+proc check_fcsr { fflags_value frm_value frm_string } {
+    # Merge fflags and frm values into a single fcsr value.
+    set fcsr_value [merge_fflags_and_frm $fflags_value $frm_value]
+
+    # Build up all the patterns we will need for this test.
+    set frm_str_re [string_to_regexp "$frm_string"]
+    set frm_val_re [format %d ${frm_value}]
+
+    set nv [format %d [expr ($fflags_value >> 4) & 0x1]]
+    set dz [format %d [expr ($fflags_value >> 3) & 0x1]]
+    set of [format %d [expr ($fflags_value >> 2) & 0x1]]
+    set uf [format %d [expr ($fflags_value >> 1) & 0x1]]
+    set nx [format %d [expr ($fflags_value >> 0) & 0x1]]
+
+    set fflags_pattern "NV:${nv} DZ:${dz} OF:${of} UF:${uf} NX:${nx}"
+    set frm_pattern "FRM:${frm_val_re} \\\[${frm_str_re}\\\]"
+    set fcsr_pattern "${fflags_pattern} ${frm_pattern}"
+
+    # Now use 'info registers' to check the register values.
+    array set reg_counts {}
+    gdb_test_multiple "info registers \$fflags \$frm \$fcsr" "" {
+       -re "^info registers\[^\r\n\]+\r\n" {
+           exp_continue
+       }
+
+       -re "^(fflags\|frm)\\s+<unavailable>\r\n" {
+           # Currently, on some targets (e.g. RISC-V native Linux) the
+           # fflags and frm registers show as being available, but are
+           # unreadable, the result is these registers report
+           # themselves as <unavailable>.  So long as fcsr is readable
+           # (which is checked below), then for now we accept this.
+           set reg_name $expect_out(1,string)
+           incr reg_counts($reg_name)
+           exp_continue
+       }
+
+       -re "^(frm)\\s+${frm_value}\\s+${frm_pattern}\r\n" {
+           set reg_name $expect_out(1,string)
+           incr reg_counts($reg_name)
+           exp_continue
+       }
+
+       -re "^(fflags)\\s+${fflags_value}\\s+${fflags_pattern}\r\n" {
+           set reg_name $expect_out(1,string)
+           incr reg_counts($reg_name)
+           exp_continue
+       }
+
+       -re "^(fcsr)\\s+${fcsr_value}\\s+${fcsr_pattern}\r\n" {
+           set reg_name $expect_out(1,string)
+           incr reg_counts($reg_name)
+           exp_continue
+       }
+
+       -re "^$::gdb_prompt $" {
+           pass $gdb_test_name
+       }
+    }
+
+    # Check that each register is seen only once.
+    foreach reg {fflags frm fcsr} {
+       gdb_assert { $reg_counts($reg) == 1 } \
+           "check we saw $reg just once"
+    }
+}
+
+# Set the fcsr register based on FFLAGS_VALUE and FRM_VALUE, then
+# check that the value is displayed correctly in the 'info registers'
+# output.  FRM_STRING should appear in the 'info registers' output
+# next to the frm field.
+proc test_fcsr { fflags_value frm_value frm_string } {
+    # Merge fflags and frm values into a single fcsr value.
+    set fcsr_value [merge_fflags_and_frm $fflags_value $frm_value]
+
+    with_test_prefix "fcsr=${fcsr_value}" {
+       # Set the fcsr value directly.
+       gdb_test_no_output "set \$fcsr = ${fcsr_value}"
+
+       with_test_prefix "set through fcsr" {
+           check_fcsr $fflags_value $frm_value $frm_string
+       }
+    }
+}
+
+# Check each valid value of the fflags register.
+for { set i 0 } { $i < 32 } { incr i } {
+    test_fcsr 0x[format %x $i] 0x0 "RNE (round to nearest; ties to even)"
+}
+
+# Check each valid value of the frm register.
+test_fcsr 0x0 0x1 "RTZ (Round towards zero)"
+test_fcsr 0x0 0x2 "RDN (Round down towards -INF)"
+test_fcsr 0x0 0x3 "RUP (Round up towards +INF)"
+test_fcsr 0x0 0x4 "RMM (Round to nearest; ties to max magnitude)"
+test_fcsr 0x0 0x5 "INVALID\[5\]"
+test_fcsr 0x0 0x6 "INVALID\[6\]"
+test_fcsr 0x0 0x7 "INVALID\[7\] (Dynamic rounding mode)"