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

[Luis Machado <luis.machado@arm.com>]

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