Re: [PATCH] [ARM] neon-testgen.ml typo

[Kyrill Tkachov <kyrylo.tkachov@arm.com>]

Hi Christophe,

On 13/11/15 10:24, Christophe Lyon wrote:
> On 12 November 2015 at 23:18, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 6 November 2015 at 21:29, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>> On 4 November 2015 at 13:16, Ramana Radhakrishnan
>>> <ramana.gcc@googlemail.com> wrote:
>>>> On Fri, Oct 30, 2015 at 2:42 PM, Christophe Lyon
>>>> <christophe.lyon@linaro.org> wrote:
>>>>> On 30 October 2015 at 15:33, Ramana Radhakrishnan
>>>>> <ramana.radhakrishnan@foss.arm.com> wrote:
>>>>>>
>>>>>> On 29/10/15 17:23, Jim Wilson wrote:
>>>>>>> I noticed a comment typo in this file while using grep to look for
>>>>>>> other stuff.  The typo is easy to fix.
>>>>>>>
>>>>>>> I tried running neon-testgen.ml to verify, but it is apparently no
>>>>>>> longer valid ocaml, as it doesn't work with the ocamlc 4.01.0 I have
>>>>>>> on Ubuntu 14.04.  I get a syntax error.  Someone who knows ocaml will
>>>>>>> have to fix this.  Meanwhile, the patch to fix the typo should still
>>>>>>> be OK, as this is a separate problem.
>>>>>>>
>>>>>>> Jim
>>>>>>>
>>>>>> This is OK.
>>>>>>
>>>>>> I'd really like neon-testgen.ml and the tests in gcc.target/arm/neon to be removed if all the intrinsics are now tested from Christophe's work in getting his advsimd tests integrated. Where are we on that ?
>>>>>>
>>>>> The tests I added cover all ARMv7 intrinsics. I have converted all my tests.
>>>> Good and thank you for doing that.
>>>>
>>>>> There were a few additions to support some aarch64 specific intrinsics.
>>>>>
>>>>> However, most of the tests in gcc.target/arm/neon contain scan-asm
>>>>> directives which mine don't.
>>>>> My tests do check functionality (they are executable, comparing their
>>>>> results against expected values).
>>>> I don't think the scan-asm in those tests gives you anything useful at
>>>> O0 with undefined behaviour in the testcases. In any case for the more
>>>> esoteric intrinsics (i.e. the ones that do saturation etc..) having
>>>> the run time tests is a better test. I do not see this as being useful
>>>> any more in terms of the testing coverage this provides.
>>> Ideally we'd need to add more tests for the new armv8 intrinsics....
>>>
>>>> A patch to remove neon-testgen.ml and the tests in gcc.target/arm/neon
>>>> is pre-approved.
>>>>
>>> OK I'l give it a look, but that will after e/o stage1 probably.
>>>
>> I have just removed them all, as r230274.
>>
>> ChangeLog is attached.
>>
> Hmm. I hadn't noticed that the crypto intrinsics tests where generated by
> neon-testgen.ml, I thought they were hand-written.
> The tests I added do not cover the crypto intrinsics, so I'm going
> to revert r230274 and restore all the tests generated by neon-testgen.ml
> until we have better coverage in advsimd-intrinsics.

We have compile tests for the crypto intrinsics in gcc.target/arm/.
Just grep for crypto in that directory. Do you think the auto-generated
tests add anything to that?

Kyrill

> Sorry for the oversight.
>
> Christophe.
>
>
>> Christophe.
>>
>>>> regards
>>>> Ramana
>>>>
>>>>
>>>>
>>>>> Christophe.
>>>>>
>>>>>> regards
>>>>>> Ramana