Re: [patch, testsuite, ARM] don't try to execute advsimd-intrinsics tests on hardware without NEON

[Richard Earnshaw <Richard dot Earnshaw at foss dot arm dot com>]

On 03/06/15 20:44, James Greenhalgh wrote:
> On Wed, Jun 03, 2015 at 08:28:12PM +0100, Sandra Loosemore wrote:
>> On 06/03/2015 12:05 PM, James Greenhalgh wrote:
>>>
>>> This has caused some issues for my multilib testing. Summarised below,
>>> with some help from Alan Lawrence.
>>>
>>> Basically the problem occurs when a target which is not OK for Neon
>>> runs before another target. The dg-do-what-default is not restored
>>> when ![check_effective_target_arm_neon_ok]. More details inline...
>>>
>>>> Index: gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp
>>>> ===================================================================
>>>> --- gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp	(revision 223468)
>>>> +++ gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp	(working copy)
>>>> @@ -32,9 +32,20 @@ load_lib torture-options.exp
>>>>
>>>>   dg-init
>>>>
>>>> -if {[istarget arm*-*-*]
>>>> -    && ![check_effective_target_arm_neon_ok]} then {
>>>> -  return
>>>> +# The default action for a test is 'run'.  Save current default.
>>>> +global dg-do-what-default
>>>> +set save-dg-do-what-default ${dg-do-what-default}
>>>> +set dg-do-what-default run
>>>
>>> This overrides the dg-do-what-default
>>>
>>>> +
>>>> +# For ARM, make sure that we have a target compatible with NEON, and do
>>>> +# not attempt to run execution tests if the hardware doesn't support it.
>>>> +if {[istarget arm*-*-*]} then {
>>>> +    if {![check_effective_target_arm_neon_ok]} then {
>>>> +      return
>>>
>>> And this return path does not restore it.
>>
>> Sigh, you are absolutely right about that.  :-(
>>
>>>
>>> Adding:
>>>
>>>> +set dg-do-what-default ${save-dg-do-what-default}
>>>
>>> before the return would seem like the right way to fix the issue.
>>>
>>> I'll spin a patch tomorrow if someone else doesn't beat me to it.
>>
>> I have a mild preference for instead not setting dg-do-what-default 
>> until we've gotten past the early return, something like the attached 
>> patch.  That's completely untested -- I've temporarily swapped out all 
>> my ARM state and am bogged down with a nios2 gdb problem right now, so 
>> if you can take it from here I'd appreciate that.
> 
> This looks sensible to me, fixes the issue, and didn't seem to cause
> any other issues in my test runs just now. I'd be happy to see this
> go in under the obvious rule - but that is your call...
> 
> Otherwise, I'm not sure where maintainership stands on these shared files,
> but if an OK from the AArch64 side is enough for you, feel free to
> commit it.
> 

I trust that you'll act sensibly and when in doubt, ask ...

R.

> Otherwise, I've added Ramana, Kyrill, Nick and Richard to CC for their
> review.
> 
> Thanks,
> James
> 
>>
>> -Sandra
>>
> 
>> Index: gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp
>> ===================================================================
>> --- gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp	(revision 224098)
>> +++ gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp	(working copy)
>> @@ -33,7 +33,6 @@ dg-init
>>  # The default action for a test is 'run'.  Save current default.
>>  global dg-do-what-default
>>  set save-dg-do-what-default ${dg-do-what-default}
>> -set dg-do-what-default run
>>  
>>  # For ARM, make sure that we have a target compatible with NEON, and do
>>  # not attempt to run execution tests if the hardware doesn't support it.
>> @@ -43,7 +42,11 @@ if {[istarget arm*-*-*]} then {
>>      }
>>      if {![is-effective-target arm_neon_hw]} then {
>>          set dg-do-what-default compile
>> +    } else {
>> +        set dg-do-what-default run
>>      }
>> +} else {
>> +    set dg-do-what-default run
>>  }
>>  
>>  set-torture-options $C_TORTURE_OPTIONS {{}} $LTO_TORTURE_OPTIONS
>