Re: [PATCH v7 17/18] [gdb/testsuite] sme: Add SVE/SME testcases

Luis Machado via Gdb-patches
2023-09-19T20:04:00Z

Hi Simon,

On 9/19/23 20:12, Simon Marchi wrote:
> On 9/18/23 17:26, Luis Machado wrote:
>> v7:
>>
>> - Bail out from sme tests if we have a remote target, due to lack of
>> support in the RSP.
>>
>> v3:
>>
>> - Additional documentation for some testcases based on upstream reviews.
>> - Addressed upstream review comments.
>>
>> v2:
>> - Introduced a couple helper functions to determine the available SVE and SME
>> vector lengths.
>>
>> - Before running a batch of tests, validate that the target supports a certain
>> SVE vector length or SME streaming vector length. If not supported, skip
>> that batch of tests.
>>
>> - Fixed up native core file testing to remove generation of a spurious core
>> file, which later would be reported as unexpected by the testsuite.
>>
>> - Fixed tpidr2 dump in core file tests.
>> --
>>
>> Add 5 SVE/SME tests to exercise all the new features like reading/writing
>> registers, pseudo-registers, signal frames and core files.
>>
>> - Sanity check for SME: Gives a brief smoke test to make sure the most basic
>> of features are working correctly.
>>
>> - ZA unavailability tests: Validates the behavior/content of the ZA register
>> is correct when no payload is available. It also exercises changing the
>> vector lengths.
>>
>> - ZA Availability tests: These tests exercise reading/writing to all the
>> possible ZA pseudo-registers, and validates the state is correct.
>>
>> - Core file tests: Validates that core file reading and writing works
>> correctly and that all state dumped/loaded is sane. This is exercised for
>> both Linux Kernel core files and gcore core files.
>>
>> - Signal frame tests: Validates the correct restoration of SME/SVE/FPSIMD
>> values across signal frames.
>>
>> Since some of these tests are very lengthy and take a little while to run
>> (under QEMU at the moment), I decided to parallelize them into smaller
>> chunks so we can throw some more CPU power at them so they run faster.
>>
>> I'd still like to add a few more tests to give the testsuite more coverage
>> in the areas of SME/SVE. Hopefully in the near future that will happen.
>>
>> Just a reminder that these SME tests are currently unsupported when gdb is
>> connected to a remote target. That's because the RSP doesn't support
>> communicating changes in vector lenghts mid-execution, so gdb will always
>> get wrong state from the remote target.
>>
>> Co-Authored-By: Ezra Sitorus <ezra.sitorus@arm.com>
>> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>
> When applying the patch, git says:
>
> .git/rebase-apply/patch:796: indent with spaces.
> unsupported "aarch64 sve/sme tests not supported for remote targets"
> .git/rebase-apply/patch:1469: indent with spaces.
> unsupported "aarch64 sve/sme tests not supported for remote targets"
> .git/rebase-apply/patch:2133: indent with spaces.
> unsupported "aarch64 sve/sme tests not supported for remote targets"
> .git/rebase-apply/patch:2870: indent with spaces.
> unsupported "aarch64 sve/sme tests not supported for remote targets"
> .git/rebase-apply/patch:3539: new blank line at EOF.
> +
> warning: 5 lines add whitespace errors.
>
> Can you please fix them?

Indeed. I remember checked things, but maybe I introduced some new whitespaces somewhere.

As you may have guessed, setting up KFAIL's was a bit of a burden, as there are a lot of
potentially failing tests. It seemed best to just make gdbserver-based testing
unsupported for now

>
> Otherwise, I did a quick check on the newly added conditions to skip the
> tests on remote, they LGTM. So with the whitespaces issue fixed:
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>

Thanks for the review!

>
> Simon