From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1726) id 8B2FE3858D39; Wed, 31 Aug 2022 15:33:00 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8B2FE3858D39 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1661959980; bh=UKafDR6IlUrfjUaYzjidJ4bfCUI+VQbC0DUBULa0q30=; h=From:To:Subject:Date:From; b=vhTPG5c4kDiLYWe5fP4FogY/zzVTwlmdtd2CVifz+dzdyQgpl/C6Jrg8vklaOtufh /QdK/Kjsh+Bho47UQssJKLIBjTK0VGwl3zJZBbOO+5zFUa3E+3/biOel2VNYcI6Grt IfgkC7UTJxgbJXppLBQBrG5fCEhvGH4yzs1Z6/X4= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Andrew Burgess To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb/riscv: improve (and fix) display of frm field in 'info registers' X-Act-Checkin: binutils-gdb X-Git-Author: Andrew Burgess X-Git-Refname: refs/heads/master X-Git-Oldrev: 6472b2302de5cd8753be629f58c8ce880d0c2e32 X-Git-Newrev: 3095d92634a938d447eed1ef2c5d59f40f44078e Message-Id: <20220831153300.8B2FE3858D39@sourceware.org> Date: Wed, 31 Aug 2022 15:33:00 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D3095d92634a9= 38d447eed1ef2c5d59f40f44078e commit 3095d92634a938d447eed1ef2c5d59f40f44078e Author: Andrew Burgess Date: Sun Aug 14 15:14:22 2022 +0100 gdb/riscv: improve (and fix) display of frm field in 'info registers' =20 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. =20 Here's how the bits of FCSR are split between FRM and FFLAGS: =20 ,--------- FFLAGS |---| 76543210 <----- FCSR |-| '--------------FRM =20 Here's how GDB currently displays these registers: =20 (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 (rou= nd to nearest; ties to even)] =20 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. =20 Additionally, the $fcsr already includes an FRM field, so the information in 'RD' is duplicated. Consider this: =20 (gdb) set $frm =3D 0x3 (gdb) info registers $fflags $frm $fcsr = =E2=94=82 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 (Rou= nd up towards +INF)] =20 See how the 'RD' field in $fflags still displays 0, while the 'RD' and 'FRM' fields in $fcsr show the same information. =20 The first change I propose in this commit is to remove the 'RD' field. After this change the output now looks like this: =20 (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)] =20 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. =20 Next, I spotted that: =20 (gdb) set $frm=3D0x7 (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 (Rou= nd up towards +INF)] =20 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. =20 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. =20 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. =20 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. =20 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. =20 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. =20 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 ''. =20 The reason why I accept 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. =20 A later patch in this series will address this issue, and will remove this check for . 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 *gdbarc= h, gdb_printf (file, "\t"); if (regnum !=3D 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 *gdba= rch, { static const char * const sfrm[] =3D { - "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 =3D ((regnum =3D=3D RISCV_CSR_FCSR_REGNUM) - ? (d >> 5) : d) & 0x3; + ? (d >> 5) : d) & 0x7; =20 gdb_printf (file, "%sFRM:%i [%s]", (regnum =3D=3D RISCV_CSR_FCSR_REGNUM diff --git a/gdb/testsuite/gdb.arch/riscv-info-fcsr.c b/gdb/testsuite/gdb.a= rch/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 . = */ + +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 . + +# 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+\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 . 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) =3D=3D 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=3D${fcsr_value}" { + # Set the fcsr value directly. + gdb_test_no_output "set \$fcsr =3D ${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)"