gdb/riscv: Fix failures on rv64 in gdb.arch/riscv-reg-aliases.exp test

Message ID 20181027094015.6472-1-andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Oct. 27, 2018, 9:40 a.m. UTC
  The gdb.arch/riscv-reg-aliases.exp test didn't take into account that
on RV64 (and RV128) the floating point registers are represented as a
union.  This patch updates the test to handle this.

Tested against RV32 and RV64.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-reg-aliases.exp: Rewrite to take account of float
	registers being unions.
---
 gdb/testsuite/ChangeLog                      |   5 +
 gdb/testsuite/gdb.arch/riscv-reg-aliases.exp | 182 +++++++++++++++++----------
 2 files changed, 121 insertions(+), 66 deletions(-)
  

Comments

Jim Wilson Oct. 29, 2018, 9:44 p.m. UTC | #1
On Sat, Oct 27, 2018 at 2:40 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>         * gdb.arch/riscv-reg-aliases.exp: Rewrite to take account of float
>         registers being unions.

I can confirm that this worked for me on my HiFive Unleashed board.
It fixes 288 failures caused by the conflict between our two patches.

Jim
  

Patch

diff --git a/gdb/testsuite/gdb.arch/riscv-reg-aliases.exp b/gdb/testsuite/gdb.arch/riscv-reg-aliases.exp
index 0b54723d8bc..b4a2c982897 100644
--- a/gdb/testsuite/gdb.arch/riscv-reg-aliases.exp
+++ b/gdb/testsuite/gdb.arch/riscv-reg-aliases.exp
@@ -29,32 +29,108 @@  if ![runto_main] then {
    return 0
 }
 
-# A list, each entry is itself a list, the first item being the
-# primary name of a register (the name GDB uses by default), and the
-# second entry being a list of register aliases.
-set register_names \
+
+# A list for all the integer register names and their aliases.  The format is
+# a list with each entry being itself a list, the first item being the primary
+# name of a register (the name GDB uses by default), and the second entry
+# being a list of register aliases.
+set xreg_names \
 { { ra {x1} } { sp {x2} } { gp {x3} } { tp {x4} } { t0 {x5} } \
   { t1 {x6} } { t2 {x7} } { fp {x8 s0} } { s1 {x9} } { a0 {x10} } \
   { a1 {x11} } { a2 {x12} } { a3 {x13} } { a4 {x14} } { a5 {x15} } \
   { a6 {x16} } { a7 {x17} } { s2 {x18} } { s3 {x19} } { s4 {x20} } \
   { s5 {x21} } { s6 {x22} } { s7 {x23} } { s8 {x24} } { s9 {x25} } \
   { s10 {x26} } { s11 {x27} } { t3 {x28} } { t4 {x29} } { t5 {x30} } \
-  { t6 {x31} } { ft0 {f0} } { ft1 {f1} } { ft2 {f2} } { ft3 {f3} } \
-  { ft4 {f4} } { ft5 {f5} } { ft6 {f6} } { ft7 {f7} } { fs0 {f8} } \
-  { fs1 {f9} } { fa0 {f10} } { fa1 {f11} } { fa2 {f12} } { fa3 {f13} } \
-  { fa4 {f14} } { fa5 {f15} } { fa6 {f16} } { fa7 {f17} } { fs2 {f18} } \
-  { fs3 {f19} } { fs4 {f20} } { fs5 {f21} } { fs6 {f22} } { fs7 {f23} } \
-  { fs8 {f24} } { fs9 {f25} } { fs10 {f26} } { fs11 {f27} } { ft8 {f28} } \
-  { ft9 {f29} } { ft10 {f30} } { ft11 {f31} } }
+  { t6 {x31} } }
+
+# This is just like XREG_NAMES, except it contains all the floating point
+# register names and their aliases.
+set freg_names \
+{ { ft0 {f0} } { ft1 {f1} } { ft2 {f2} } { ft3 {f3} } { ft4 {f4} } \
+  { ft5 {f5} } { ft6 {f6} } { ft7 {f7} } { fs0 {f8} } { fs1 {f9} } \
+  { fa0 {f10} } { fa1 {f11} } { fa2 {f12} } { fa3 {f13} } { fa4 {f14} } \
+  { fa5 {f15} } { fa6 {f16} } { fa7 {f17} } { fs2 {f18} } { fs3 {f19} } \
+  { fs4 {f20} } { fs5 {f21} } { fs6 {f22} } { fs7 {f23} } { fs8 {f24} } \
+  { fs9 {f25} } { fs10 {f26} } { fs11 {f27} } { ft8 {f28} } { ft9 {f29} } \
+  { ft10 {f30} } { ft11 {f31} } }
 
 # Check that the zero register (and its x0 alias) both contain the
 # value 0.
-
 proc check_zero_register_value {testname} {
     gdb_test "p/d \$zero" " = 0" "check \$zero: ${testname}"
     gdb_test "p/d \$x0" " = 0" "check \$x0: ${testname}"
 }
 
+# Set all of the registers in REG_SET to zero.  Confirm that the value of zero
+# can be read back using the primary name, and from all of the alias names.
+#
+# For some architectures (RV64, RV128) the float registers have union type,
+# and we need to read/write using a ".float" extension.  This is passed in
+# REG_EXTENSION.  If no extension is needed then REG_EXTENSION is the empty
+# string.
+proc check_setting_registers_to_zero { reg_set reg_extension } {
+    foreach reg_desc ${reg_set} {
+	set primary_name [lindex ${reg_desc} 0]
+	set alias_names [lindex ${reg_desc} 1]
+
+	gdb_test_no_output "set \$${primary_name}${reg_extension} = 0" \
+	    "set register ${primary_name} to an initial value of zero"
+	gdb_test "p/d \$${primary_name}${reg_extension}" " = 0" \
+	    "check the initial value of ${primary_name} is now zero"
+
+	foreach reg_alias ${alias_names} {
+	    gdb_test "p/d \$${reg_alias}${reg_extension}" " = 0" \
+		"check the initial value of ${reg_alias} is now zero"
+	}
+    }
+}
+
+# Set all of the registers in REG_SET to a new value (the value starts at
+# REG_VALUE and is incremented after each test).  Then confirm that the new
+# value can be read back using the primary name, and from all of the alias
+# names.
+#
+# Next, set each register in REG_SET using each of its alias names, then
+# confirm that the value can be read back using both the primary name, and all
+# of the aliases.
+#
+# The REG_EXTENSION field is used as in CHECK_SETTING_REGISTERS_TO_ZERO.
+proc check_setting_registers_to_value { reg_set reg_extension reg_value } {
+    foreach reg_desc ${reg_set} {
+	set primary_name [lindex ${reg_desc} 0]
+	set alias_names [lindex ${reg_desc} 1]
+
+	# Set value through the primary register name, and check that all
+	# the aliases see the same value.
+	set reg_value [incr reg_value]
+	gdb_test_no_output "set \$${primary_name}${reg_extension} = $reg_value" \
+	    "write non-zero value to ${primary_name}"
+	gdb_test "p/d \$${primary_name}${reg_extension}" " = $reg_value" \
+	    "read ${primary_name} after non-zero write to ${primary_name}"
+	foreach reg_alias ${alias_names} {
+	    gdb_test "p/d \$${reg_alias}${reg_extension}" " = $reg_value" \
+		"read ${reg_alias} after non-zero write to ${primary_name}"
+	}
+
+	# For each alias, set a new value, and check that the primary
+	# register name, and all the other aliases, see the new value.
+	foreach reg_alias ${alias_names} {
+	    set reg_value [incr reg_value]
+
+	    gdb_test_no_output "set \$${reg_alias}${reg_extension} = $reg_value" \
+		"write non-zero value to ${reg_alias}"
+
+	    gdb_test "p/d \$${primary_name}${reg_extension}" " = $reg_value" \
+		"read ${primary_name} after non-zero write to ${reg_alias}"
+
+	    foreach other_reg_alias ${alias_names} {
+		gdb_test "p/d \$${other_reg_alias}${reg_extension}" " = $reg_value" \
+		    "read ${other_reg_alias} after non-zero write to ${reg_alias}"
+	    }
+	}
+    }
+}
+
 # First, some testing of the zero register.  This register should
 # always read as zero, and should swallow any attempt to write a
 # non-zero value to the register.
@@ -71,60 +147,34 @@  gdb_test_no_output "set \$x0 = 123" \
 
 check_zero_register_value "after write to \$x0"
 
-# Set all of the general registers to zero.  Confirm that the value of
-# zero can be read back from the primary name, and from all of the
-# alias names.
-
-foreach reg_desc ${register_names} {
-    set primary_name [lindex ${reg_desc} 0]
-    set alias_names [lindex ${reg_desc} 1]
-
-    gdb_test_no_output "set \$${primary_name} = 0" \
-	"set register ${primary_name} to an initial value of zero"
-    gdb_test "p/d \$${primary_name}" " = 0" \
-	"check the initial value of ${primary_name} is now zero"
-
-    foreach reg_alias ${alias_names} {
-	gdb_test "p/d \$${reg_alias}" " = 0" \
-	    "check the initial value of ${reg_alias} is now zero"
+# Some RISC-V variants model the fregs as a union (RV64, RV128).  In this case
+# we should access the register using 'REG_NAME.float'.  In the following we
+# figure out if the field name is needed or not by looking at how GDB prints
+# on register.
+set freg_extension "INVALID"
+set message "check format of float registers"
+gdb_test_multiple "p \$ft0" $message {
+    -re " = {float = \[^\r\n\]+}\r\n$gdb_prompt $" {
+	set freg_extension ".float"
+	pass $message
     }
-}
-
-# Set each register in turn to a new value, and confirm that the new
-# value can be read back from the primary name, and from all of the
-# alias names.
-
-set reg_value 100
-foreach reg_desc ${register_names} {
-    set primary_name [lindex ${reg_desc} 0]
-    set alias_names [lindex ${reg_desc} 1]
-
-    # Set value through the primary register name, and check that all
-    # the aliases see the same value.
-    set reg_value [incr reg_value]
-    gdb_test_no_output "set \$${primary_name} = $reg_value" \
-	"write non-zero value to ${primary_name}"
-    gdb_test "p/d \$${primary_name}" " = $reg_value" \
-	"read ${primary_name} after non-zero write to ${primary_name}"
-    foreach reg_alias ${alias_names} {
-	gdb_test "p/d \$${reg_alias}" " = $reg_value" \
-	    "read ${reg_alias} after non-zero write to ${primary_name}"
-    }
-
-    # For each alias, set a new value, and check that the primary
-    # register name, and all the other aliases, see the new value.
-    foreach reg_alias ${alias_names} {
-	set reg_value [incr reg_value]
-
-	gdb_test_no_output "set \$${reg_alias} = $reg_value" \
-	    "write non-zero value to ${reg_alias}"
-
-	gdb_test "p/d \$${primary_name}" " = $reg_value" \
-	    "read ${primary_name} after non-zero write to ${reg_alias}"
-
-	foreach other_reg_alias ${alias_names} {
-	    gdb_test "p/d \$${other_reg_alias}" " = $reg_value" \
-		"read ${other_reg_alias} after non-zero write to ${reg_alias}"
-	}
+    -re " = \[^{}\r\n\]+\r\n$gdb_prompt $" {
+	set freg_extension ""
+	pass $message
     }
 }
+gdb_assert ![string eq "${freg_extension}" "INVALID"] \
+    "check that floating point format has been understood"
+
+# Now check that we can write zero, and read zero back to all of the integer
+# and floating point registers.
+check_setting_registers_to_zero ${xreg_names} ""
+check_setting_registers_to_zero ${freg_names} ${freg_extension}
+
+# Set each register in turn to a new value, and confirm that the new value can
+# be read back from the primary name, and from all of the alias names.  The
+# value passed in to each test invocation here is arbitrary, they are
+# significantly different so that the float tests don't reuse value from the
+# integer tests.
+check_setting_registers_to_value ${xreg_names} "" 100
+check_setting_registers_to_value ${freg_names} ${freg_extension} 500