Re: [RFA] Fix setting of VSX registers

Thiago Jung Bauermann
2010-07-26T17:31:41Z

[-- Attachment #1: Type: text/plain, Size: 2810 bytes --]

On Thu, 2010-07-22 at 10:05 -0600, Tom Tromey wrote:
> >>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:
>
> Thiago> The problem is, the function was "fetching" the VSX registers using
> Thiago> PTRACE_SETVSXREGS instead of PTRACE_GETVSXREGS. Ouch.
>
> I don't know this code at all, but this change seems obviously ok to me.

Indeed. I just applied this part to HEAD. Is the obvious rule valid for
the 7.2 branch too?

Joel, would you mind me committing this hunk to the 7.2 branch?

--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -877,7 +877,7 @@ store_vsx_register (const struct regcache *regcache, int tid, int regno)
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
int vsxregsize = register_size (gdbarch, tdep->ppc_vsr0_upper_regnum);

- ret = ptrace (PTRACE_SETVSXREGS, tid, 0, &regs);
+ ret = ptrace (PTRACE_GETVSXREGS, tid, 0, &regs);
if (ret < 0)
{
if (errno == EIO)

> Thiago> This patch fixes the typo, and also fixes the vsx-regs.exp testcase to
> Thiago> use gdb_test instead of send_gdb (this also fixes some synchronization
> Thiago> issues in the test), and updates the expect info reg output with the new
> Thiago> v2_double member.
>
> I don't understand why the new gdb_test calls have an empty "message"
> argument.

So that they don't increase the test count. They are just sending
command to GDB to set the stage for the actual tests, they're of no
intrinsic interest to the testcase.

> Actually, this code in gdb_test itself looks somewhat bogus.
> Aside from parsing arguments by hand (why??), it uses a different
> default for the message than gdb_test_multiple. I don't understand
> when this can ever be the right thing to do.

I agree it's strange, but TCL looks very alien to my eyes and I don't
tend to question the testsuite way of doing things. :-)

> For your patch I suggest just leaving off the 3rd argument.

In that case I wouldn't get the "this is not an actual test" effect that
I'm interested in.

> Also, when the second argument to gdb_test is the empty string ... I
> suspect you actually want to use gdb_test_no_output.

Indeed. I was under the impression that there was a specific function
for that but I couldn't find it. I just updated the GDBTestcaseCookbook
wiki page with that information.

Here's new version of the testcase patch updated to use
gdb_test_no_output, but keeping the empty message argument. WDYT?
--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

2010-07-26 Thiago Jung Bauermann <bauerman@br.ibm.com>

* gdb.arch/vsx-regs.exp: Remove wrong comment about testing AltiVec
registers. Update data sets with the new v2_double element in the VSX
register union. Add vector_register3_vr data set for the AltiVec
registers. Use gdb_test_no_output instead of send_gdb.

[-- Attachment #2: vsx.diff --]
[-- Type: text/x-patch, Size: 4424 bytes --]

diff --git a/gdb/testsuite/gdb.arch/vsx-regs.exp b/gdb/testsuite/gdb.arch/vsx-regs.exp
index f310a6f..8cd8ebe 100644
--- a/gdb/testsuite/gdb.arch/vsx-regs.exp
+++ b/gdb/testsuite/gdb.arch/vsx-regs.exp
@@ -14,8 +14,6 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

-# Tests for Powerpc AltiVec register setting and fetching
-
if $tracelevel then {
strace $tracelevel
}
@@ -66,11 +64,13 @@ if ![runto_main] then {

# Data sets used throughout the test

-set vector_register1 ".uint128 = 0x3ff4cccccccccccc0000000000000000, v4_float = .0x1, 0xf99999a0, 0x0, 0x0., v4_int32 = .0x3ff4cccc, 0xcccccccc, 0x0, 0x0., v8_int16 = .0x3ff4, 0xcccc, 0xcccc, 0xcccc, 0x0, 0x0, 0x0, 0x0., v16_int8 = .0x3f, 0xf4, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0.."
+set vector_register1 ".uint128 = 0x3ff4cccccccccccc0000000000000000, v2_double = .0x1, 0x0., v4_float = .0x1, 0xf99999a0, 0x0, 0x0., v4_int32 = .0x3ff4cccc, 0xcccccccc, 0x0, 0x0., v8_int16 = .0x3ff4, 0xcccc, 0xcccc, 0xcccc, 0x0, 0x0, 0x0, 0x0., v16_int8 = .0x3f, 0xf4, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0.."
+
+set vector_register2 "uint128 = 0xdeadbeefdeadbeefdeadbeefdeadbeef, v2_double = .0x1, 0x1., v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef., v8_int16 = .0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef., v16_int8 = .0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef.."

-set vector_register2 "uint128 = 0xdeadbeefdeadbeefdeadbeefdeadbeef, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef., v8_int16 = .0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef., v16_int8 = .0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef.."
+set vector_register3 ".uint128 = 0x00000001000000010000000100000001, v2_double = .0x0, 0x0., v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."

-set vector_register3 ".uint128 = 0x00000001000000010000000100000001, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
+set vector_register3_vr ".uint128 = 0x00000001000000010000000100000001, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."

set float_register ".raw 0xdeadbeefdeadbeef."

@@ -78,7 +78,7 @@ set float_register ".raw 0xdeadbeefdeadbeef."

# 1: Set F0~F31 registers and check if it reflects on VS0~VS31.
for {set i 0} {$i < 32} {incr i 1} {
- send_gdb "set \$f$i = 1\.3"
+ gdb_test_no_output "set \$f$i = 1\.3" ""
}

for {set i 0} {$i < 32} {incr i 1} {
@@ -88,7 +88,7 @@ for {set i 0} {$i < 32} {incr i 1} {
# 2: Set VS0~VS31 registers and check if it reflects on F0~F31.
for {set i 0} {$i < 32} {incr i 1} {
for {set j 0} {$j < 4} {incr j 1} {
- send_gdb "set \$vs$i.v4_int32\[$j\] = 0xdeadbeef"
+ gdb_test_no_output "set \$vs$i.v4_int32\[$j\] = 0xdeadbeef" ""
}
}

@@ -105,7 +105,7 @@ for {set i 0} {$i < 32} {incr i 1} {
# 1: Set VR0~VR31 registers and check if it reflects on VS32~VS63.
for {set i 0} {$i < 32} {incr i 1} {
for {set j 0} {$j < 4} {incr j 1} {
- send_gdb "set \$vr$i.v4_int32\[$j\] = 1"
+ gdb_test_no_output "set \$vr$i.v4_int32\[$j\] = 1" ""
}
}

@@ -115,12 +115,12 @@ for {set i 32} {$i < 64} {incr i 1} {
# 2: Set VS32~VS63 registers and check if it reflects on VR0~VR31.
for {set i 32} {$i < 64} {incr i 1} {
for {set j 0} {$j < 4} {incr j 1} {
- send_gdb "set \$vs$i.v4_int32\[$j\] = 1"
+ gdb_test_no_output "set \$vs$i.v4_int32\[$j\] = 1" ""
}
}

for {set i 0} {$i < 32} {incr i 1} {
- gdb_test "info reg vr$i" "vr$i.*$vector_register3" "info reg vr$i"
+ gdb_test "info reg vr$i" "vr$i.*$vector_register3_vr" "info reg vr$i"
}

set escapedfilename [string_to_regexp ${objdir}/${subdir}/vsx-core.test]