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

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.

-Sandra

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.

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

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
>