public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [AArch64] Fix SVE-related failure in gdb.arch/aarch64-fp.exp
@ 2020-02-26 14:57 Luis Machado
  2020-02-27 16:38 ` Alan Hayward
  0 siblings, 1 reply; 5+ messages in thread
From: Luis Machado @ 2020-02-26 14:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: Alan.Hayward

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.

(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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [AArch64] Fix SVE-related failure in gdb.arch/aarch64-fp.exp
  2020-02-26 14:57 [PATCH] [AArch64] Fix SVE-related failure in gdb.arch/aarch64-fp.exp Luis Machado
@ 2020-02-27 16:38 ` Alan Hayward
  2020-02-27 19:49   ` Luis Machado
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Hayward @ 2020-02-27 16:38 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches, nd



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

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.

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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [AArch64] Fix SVE-related failure in gdb.arch/aarch64-fp.exp
  2020-02-27 16:38 ` Alan Hayward
@ 2020-02-27 19:49   ` Luis Machado
  2020-02-28 13:27     ` Alan Hayward
  0 siblings, 1 reply; 5+ messages in thread
From: Luis Machado @ 2020-02-27 19:49 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

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.

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?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [AArch64] Fix SVE-related failure in gdb.arch/aarch64-fp.exp
  2020-02-27 19:49   ` Luis Machado
@ 2020-02-28 13:27     ` Alan Hayward
  2020-02-28 15:25       ` Luis Machado
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Hayward @ 2020-02-28 13:27 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches, nd



> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [AArch64] Fix SVE-related failure in gdb.arch/aarch64-fp.exp
  2020-02-28 13:27     ` Alan Hayward
@ 2020-02-28 15:25       ` Luis Machado
  0 siblings, 0 replies; 5+ messages in thread
From: Luis Machado @ 2020-02-28 15:25 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

On 2/28/20 10:27 AM, Alan Hayward wrote:
> 
> 
>> 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.

Thanks. Pushed now.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-02-28 15:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-26 14:57 [PATCH] [AArch64] Fix SVE-related failure in gdb.arch/aarch64-fp.exp Luis Machado
2020-02-27 16:38 ` Alan Hayward
2020-02-27 19:49   ` Luis Machado
2020-02-28 13:27     ` Alan Hayward
2020-02-28 15:25       ` Luis Machado

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