Re: [Patch ARM-AArch64/testsuite v2 01/21] Neon intrinsics execution tests initial framework.

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On 1 October 2014 17:11, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote:
> On 30 September 2014 15:27, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>> On 10 July 2014 12:12, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote:
>>> On 1 July 2014 11:05, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>> * documentation (README)
>>>> * dejanu driver (neon-intrinsics.exp)
>>>> * support macros (arm-neon-ref.h, compute-ref-data.h)
>>>> * Tests for 3 intrinsics: vaba, vld1, vshl
>>>
>>> Hi, The terminology in armv8 is advsimd rather than neon.  Can we
>>> rename neon-intrinsics to advsimd-intrinsics or simd-intrinsics
>>> throughout please.  The existing gcc.target/aarch64/simd directory of
>>> tests will presumably be superseded by this more comprehensive set of
>>> tests so I suggest these tests go in gcc.target/aarch64/advsimd and we
>>> eventually remove gcc.target/aarch64/simd/ directory.
>>>
>>> GNU style should apply throughout this patch series, notably double
>>> space after period in comments and README text.  Space before left
>>> parenthesis in function/macro call and function declaration.  The
>>> function name in a declaration goes on a new line.  The GCC wiki notes
>>> on test case state individual test should have file names ending in
>>> _<number>, see here https://gcc.gnu.org/wiki/TestCaseWriting
>>>
>>
>> Hi,
>>
>> For the record, these tests are based on a testsuite I wrote quite
>> some time ago:
>> https://gitorious.org/arm-neon-tests/
>>
>> where obviously I had no such requirement (and v8 wasn't public yet)
>>
>> So I prefer to apply the changes you request in my main version before
>> re-submitting it here.
>> (libsanitizer-style, sort-of....).
>>
>> This will take me some time, so the next version of my patch series
>> should not be expected really soon :-(
>
>
Ramana, Marcus,

> Hi Christophe,   Given that this test suite code is an existing body
> of work I see no reason to impose the GNU style change I originally
> asked for. I withdraw my original comment that these patches should
> conform to GNU style.  My comment on file names is also withdrawn.  I
> would like to see the terminology corrected.
>

Thanks, I have updated my patch according to this.

But meanwhile I have also updated my testsuite, and fixed the #define
flag I used to toggle float16 tests: I now use __ARM_FP16_FORMAT_IEEE,
such as:
#if defined(__ARM_FP16_FORMAT_IEEE)
  TEST_VLD1(vector, buffer, , float, f, 16, 4);
  TEST_VLD1(vector, buffer, q, float, f, 16, 8);
#endif

Which reminded me that:
- on ARM (AArch32), float16x4_t is supported, but float16x8_t isn't yet
- on AArch64, -mfp16-format=ieee is rejected, and I didn't see a
similar option in the doc

What do you prefer me to do for these tests? I can think of:
- do not include them at all until fp16 is fully supported on both
AArch32 and AArch64
- include only those with float16x4_t
- include both float16x4_t and float16x8_t tests, leaving float16x8_t commented
- include both, uncommented, but do not test with -mfp16-format=ieee

Thanks,

Christophe.


> Thanks
> /Marcus