Re: [PATCH] [AArch64] Fix SVE-related failure in gdb.arch/aarch64-fp.exp

[Alan Hayward <Alan dot Hayward at arm dot com>]

> On 27 Feb 2020, at 19:49, Luis Machado <luis.machado@linaro.org> wrote:
> 
> Hi,
> 
> On 2/27/20 1:37 PM, Alan Hayward wrote:
>>> On 26 Feb 2020, at 14:57, Luis Machado <luis.machado@linaro.org> wrote:
>>> 
>>> The gdb.arch/aarch64-fp.exp test assumes it is dealing with a regular SIMD
>>> target that exposes the V registers as raw registers.  SVE-enabled targets
>>> turn the V registers into pseudo-registers.
>>> 
>>> That is all fine, but the testcase uses the "info registers" command, which
>>> prints pseudo-register's contents twice. One for the hex format and another
>>> for the natural format of the type.
>>> 
>> I find it very odd behaviour that it switches output style.
>> > Looking at AArch64 and X86, it looks like the behaviour is for non vector
>> registers to be printed in hex and dec, and vector registers just printed in
>> hex (probably to save space).
> 
> Indeed that seems to be the main reason, given this patch from 2002:
> 
> https://sourceware.org/ml/gdb-patches/2002-08/msg00539.html
> 
> 18 years later, I'm not sure we want to keep printing the contents twice. We can easily switch to whatever format we want. But this may require further discussion (and adjustment of testcases). I'll put this in the backlog of things to bring up.
> 

Definitely once for vector registers.

> With that said, however,...
> 
>> For consistency between SVE and non-SVE, it’d make sense if the V registers
>> were printed the same way. Not sure how trivial or tricky that’d be to change. >
>> I guess there is an argument that Q/D/S/H/B registers should only be printed
>> in hex too. But given the output here is short, I could be convinced either
>> way.
> 
> ... what we want to test with gdb.arch/aarch64-fp.exp is if the contents of a particular register are sane and if we can access them. It seems the 'p' command is a bit more reliable in this regard, and also allows different formats to be specified.
> 
> What do you think?

Yes, agreed, especially since your changes are forcing them to hex output too.

Ok to push.

Alan.

> 
>> Alan.
>>> (gdb) info registers v0
>>> v0             {d = {f = {0x0, 0x0}, u = {0x1716151413121110, 0x1f1e1d1c1b1a1918}, s = {0x1716151413121110, 0x1f1e1d1c1b1a1918}}, s = {f = {0x0, 0x0, 0x0, 0x0}, u = {0x13121110, 0x17161514, 0x1b1a1918, 0x1f1e1d1c}, s = {0x13121110, 0x17161514, 0x1b1a1918, 0x1f1e1d1c}}, h = {f = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, u = {0x1110, 0x1312, 0x1514, 0x1716, 0x1918, 0x1b1a, 0x1d1c, 0x1f1e}, s = {0x1110, 0x1312, 0x1514, 0x1716, 0x1918, 0x1b1a, 0x1d1c, 0x1f1e}}, b = {u = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, s = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}}, q = {u = {0x1f1e1d1c1b1a19181716151413121110}, s = {0x1f1e1d1c1b1a19181716151413121110}}}
>>> {d = {f = {1.846323925681849e-197, 8.5677456166123577e-159}, u = {1663540288323457296, 2242261671028070680}, s = {1663540288323457296, 2242261671028070680}}, s = {f = {1.84362032e-27, 4.84942184e-25, 1.27466897e-22, 3.34818801e-20}, u = {319951120, 387323156, 454695192, 522067228}, s = {319951120, 387323156, 454695192, 522067228}}, h = {f = {0.00061798, 0.00086308, 0.0012398, 0.00173, 0.0024872, 0.0034676, 0.0049896, 0.0069504}, u = {4368, 4882, 5396, 5910, 6424, 6938, 7452, 7966}, s = {4368, 4882, 5396, 5910, 6424, 6938, 7452, 7966}}, b = {u = {16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31}, s = {16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31}}, q = {u = {41362427191743139026751447860679676176}, s = {41362427191743139026751447860679676176}}}
>>> 
>>> (gdb) p/x $v0
>>> $1 = {d = {f = {0x0, 0x0}, u = {0x1716151413121110, 0x1f1e1d1c1b1a1918}, s = {0x1716151413121110, 0x1f1e1d1c1b1a1918}}, s = {f = {0x0, 0x0, 0x0, 0x0}, u = {0x13121110, 0x17161514, 0x1b1a1918, 0x1f1e1d1c}, s = {0x13121110, 0x17161514, 0x1b1a1918, 0x1f1e1d1c}}, h = {f = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, u = {0x1110, 0x1312, 0x1514, 0x1716, 0x1918, 0x1b1a, 0x1d1c, 0x1f1e}, s = {0x1110, 0x1312, 0x1514, 0x1716, 0x1918, 0x1b1a, 0x1d1c, 0x1f1e}}, b = {u = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, s = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}}, q = {u = {0x1f1e1d1c1b1a19181716151413121110}, s = {0x1f1e1d1c1b1a19181716151413121110}}}
>>> 
>>> Since the testcase is not expecting that, we run into a couple failures:
>>> 
>>> FAIL: gdb.arch/aarch64-fp.exp: check register v0 value
>>> FAIL: gdb.arch/aarch64-fp.exp: check register v1 value
>>> 
>>> The following patch switches to using "p/x" for printing register values, which
>>> prints the values once with the hex format, instead of twice.
>>> 
>>> gdb/testsuite/ChangeLog
>>> 
>>> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
>>> 
>>> 	* gdb.arch/aarch64-fp.exp: Switch from "info registers" command
>>> 	to "p/x".
>>> 
>>> Signed-off-by: Luis Machado <luis.machado@linaro.org>
>>> ---
>>> gdb/testsuite/gdb.arch/aarch64-fp.exp | 12 ++++++------
>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-fp.exp b/gdb/testsuite/gdb.arch/aarch64-fp.exp
>>> index 8ba3377317..74224d48bd 100644
>>> --- a/gdb/testsuite/gdb.arch/aarch64-fp.exp
>>> +++ b/gdb/testsuite/gdb.arch/aarch64-fp.exp
>>> @@ -51,27 +51,27 @@ if {$endianness == "little"} {
>>>     set reg_value1 "0x202122232425262728292a2b2c2d2e2f"
>>> }
>>> 
>>> -gdb_test "info registers q0" \
>>> +gdb_test "p/x \$q0" \
>>>     "q0.*{u = $reg_value0, s = $reg_value0.*" \
>>>     "check register q0 value"
>>> 
>>> -gdb_test "info registers q1" \
>>> +gdb_test "p/x \$q1" \
>>>     "q1.*{u = $reg_value1, s = $reg_value1.*" \
>>>     "check register q1 value"
>>> 
>>> -gdb_test "info registers v0" \
>>> +gdb_test "p/x \$v0" \
>>>     "v0.*$reg_value0}}}" \
>>>     "check register v0 value"
>>> 
>>> -gdb_test "info registers v1" \
>>> +gdb_test "p/x \$v1" \
>>>     "v1.*$reg_value1}}}" \
>>>     "check register v1 value"
>>> 
>>> -gdb_test "info registers fpsr" \
>>> +gdb_test "p/x \$fpsr" \
>>>     "fpsr.*0x\[0-9a-fA-F\].*" \
>>>     "check register fpsr value"
>>> 
>>> -gdb_test "info registers fpcr" \
>>> +gdb_test "p/x \$fpcr" \
>>>     "fpcr.*0x\[0-9a-fA-F\].*" \
>>>     "check register fpcr value"
>>> 
>>> -- 
>>> 2.17.1