public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb/riscv: improve (and fix) display of frm field in 'info registers'
@ 2022-08-31 15:33 Andrew Burgess
  0 siblings, 0 replies; only message in thread
From: Andrew Burgess @ 2022-08-31 15:33 UTC (permalink / raw)
  To: gdb-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3095d92634a938d447eed1ef2c5d59f40f44078e

commit 3095d92634a938d447eed1ef2c5d59f40f44078e
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sun Aug 14 15:14:22 2022 +0100

    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:
---
 gdb/riscv-tdep.c                           |  24 ++---
 gdb/testsuite/gdb.arch/riscv-info-fcsr.c   |  22 +++++
 gdb/testsuite/gdb.arch/riscv-info-fcsr.exp | 147 +++++++++++++++++++++++++++++
 3 files changed, 182 insertions(+), 11 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 9ec430d8a10..93ee597af58 100644
--- 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
index 00000000000..034c190692a
--- /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
index 00000000000..3d6095d7e55
--- /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)"

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-08-31 15:33 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-31 15:33 [binutils-gdb] gdb/riscv: improve (and fix) display of frm field in 'info registers' Andrew Burgess

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).