Re: [PINGv4][PATCHv3] Fix vector tests on ARM platforms with disabled unaligned accesses

[Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>]

On Thu, Aug 21, 2014 at 4:05 PM, Marat Zakirov <m.zakirov@samsung.com> wrote:
>
> On 08/14/2014 05:40 PM, Marat Zakirov wrote:
>>
>>
>> On 08/07/2014 12:52 PM, Marat Zakirov wrote:
>>>
>>>
>>> On 07/31/2014 04:08 PM, Marat Zakirov wrote:
>>>>
>>>>
>>>> On 07/24/2014 07:40 PM, Marat Zakirov wrote:
>>>>>
>>>>>
>>>>> On 07/24/2014 04:27 PM, Marat Zakirov wrote:
>>>>>>
>>>>>>
>>>>>> On 07/23/2014 06:23 PM, Marat Zakirov wrote:
>>>>>>>
>>>>>>> Hi there!
>>>>>>>
>>>>>>> I made a patch which fixes regressions on ARM platforms with disabled
>>>>>>> unaligned accesses. The problem is that 'arm_vect_no_misalign' predicate do
>>>>>>> not check 'unaligned_access' global variable to determine whether unaligned
>>>>>>> access to vector are allowed. This leads to spurious vect.exp test fails
>>>>>>> when GCC is configured
>>>>>>> --with-specs=%{!munaligned-access:-mno-unaligned-access}.
>>>>>>>
>>>>>>> Attached patch fixes ARM predicate and several tests to correctly
>>>>>>> handle the issue.
>>>>>>>
>>>>>>> The following targets were reg. tested for multiple targets (ARM,
>>>>>>> Thumb-1, Thumb-2, x86, x86_64) with and without -mno-unaligned-access.
>>>>>>> Analysis showed patch affects only vect.exp tests so only vect.exp was
>>>>>>> tested.
>>>>>>>
>>>>>>> For x86, x86_64,  ARM without -mno-unaligned-access, Thumb-2 without
>>>>>>> -mno-unaligned-access and Thumb-1 no regressions occured. For ARM/Thumb2
>>>>>>> with -mno-unaligned-access patch fixed most of failures but triggered some
>>>>>>> problems (see attached log) for current vect.exp tests:
>>>>>>> 1) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61887
>>>>>>> 2) Some XPASS'es due to unexpected loop versioning (e.g.
>>>>>>> gcc.dg/vect/pr33804.c).
>>>>>>> 3) After predicate fix some passing tests which require unaligned
>>>>>>> vector support become NA (this was expected).
>>>>>>>
>>>>>> Here is new version of patch and regression log. On the current trunk
>>>>>> results are slightly different due to patches for Richard Biener (no
>>>>>> UNRESOLVED fails) but some PASS->XPASS regressions still remain (see
>>>>>> attachment):
>>>>>>
>>>>>> PASS->XPASS: gcc.dg/vect/if-cvt-stores-vect-ifcvt-18.c
>>>>>> scan-tree-dump-times vect "vectorized 1 loops" 1
>>>>>> PASS->XPASS: gcc.dg/vect/pr33804.c -flto -ffat-lto-objects
>>>>>> scan-tree-dump-times vect "vectorized 1 loops" 1
>>>>>> etc.
>>>>>>
>>>>>> These XPASS'es are due to code versioning: current GCC creates 2
>>>>>> versions of loop: aligned and misaligned. It's look like they are slightly
>>>>>> out of date at lest for ARM.
>>>>>>
>>>>>
>>>>> On 07/24/2014 06:50 PM, Ramana Radhakrishnan wrote:
>>>>>>
>>>>>> This is redundant.
>>>>>>
>>>>>>> -           || (defined(__ARMEL__) \
>>>>>>> +           || (defined(__ARM_FEATURE_UNALIGNED) \
>>>>>>> +               && defined(__ARMEL__) \
>>>>>>>                  && (!defined(__thumb__) || defined(__thumb2__)))
>>>>>>
>>>>>> As is this line.
>>>>>>
>>>>>> I think you can restrict the check to defined(__ARM_FEATURE_UNALIGNED)
>>>>>> && defined(__ARMEL__)
>>>>>>
>>>>>>   __ARM_FEATURE_UNALIGNED should tell you whether unaligned access is
>>>>>> allowed or not, therefore you should no longer require any specific
>>>>>> "architectural" checks.
>>>>>>
>>>>>>
>>>>>>>          #error FOO
>>>>>>>          #endif
>>>>>>>
>>>>>> I'm not sure about the original intent of the tests right now.
>>>>>>
>>>>>> Ramana
>>>>>>
>>>>>
>>>>> Thank you Ramana!
>>>>>
>>>>> --Marat
>>>>
>>>>
>>>
>>
>
>
> gcc/testsuite/ChangeLog:
>
> 2014-07-23  Marat Zakirov  <m.zakirov@samsung.com>
>
>         * gcc.dg/vect/vect-109.c: Skip predicate added.
>         * gcc.dg/vect/vect-93.c: Test check fixed.
>         * gcc.dg/vect/bb-slp-10.c: Likewise.
>         * lib/target-supports.exp
> (check_effective_target_arm_vect_no_misalign):
>         Check unaligned feature.
>
> diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-10.c
> b/gcc/testsuite/gcc.dg/vect/bb-slp-10.c
> index a1850ed..0090a4b 100644
> --- a/gcc/testsuite/gcc.dg/vect/bb-slp-10.c
> +++ b/gcc/testsuite/gcc.dg/vect/bb-slp-10.c
> @@ -49,7 +49,7 @@ int main (void)
>    return 0;
>  }
>
> -/* { dg-final { scan-tree-dump-times "unsupported alignment in basic
> block." 1 "slp2" { xfail vect_element_align } } } */
> +/* { dg-final { scan-tree-dump "unsupported alignment in basic block." 1
> "slp2" { xfail vect_element_align } } } */
>  /* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp2" {
> target vect_element_align } } } */
>  /* { dg-final { cleanup-tree-dump "slp2" } } */
>
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-109.c
> b/gcc/testsuite/gcc.dg/vect/vect-109.c
> index 854c970..c671175 100644
> --- a/gcc/testsuite/gcc.dg/vect/vect-109.c
> +++ b/gcc/testsuite/gcc.dg/vect/vect-109.c
> @@ -1,3 +1,4 @@
> +/* { dg-skip-if "" { vect_no_align } } */
>  /* { dg-require-effective-target vect_int } */
>
>  #include <stdarg.h>
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-93.c
> b/gcc/testsuite/gcc.dg/vect/vect-93.c
> index 65403eb..1065a6e 100644
> --- a/gcc/testsuite/gcc.dg/vect/vect-93.c
> +++ b/gcc/testsuite/gcc.dg/vect/vect-93.c
> @@ -79,7 +79,7 @@ int main (void)
>  /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect" { target
> vect_no_align } } } */
>
>  /* in main: */
> -/* { dg-final { scan-tree-dump-times "vectorized 0 loops" 1 "vect" { target
> vect_no_align } } } */
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target
> vect_no_align } } } */
>  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1
> "vect" { xfail { vect_no_align } } } } */
>
>  /* { dg-final { cleanup-tree-dump "vect" } } */
> diff --git a/gcc/testsuite/lib/target-supports.exp
> b/gcc/testsuite/lib/target-supports.exp
> index db65ebe..35076d2 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -2293,8 +2293,8 @@ proc check_effective_target_arm_little_endian { } {
>  proc check_effective_target_arm_vect_no_misalign { } {
>      return [check_no_compiler_messages arm_vect_no_misalign assembly {
>         #if !defined(__arm__) \
> -           || (defined(__ARMEL__) \
> -               && (!defined(__thumb__) || defined(__thumb2__)))
> +           || (defined(__ARM_FEATURE_UNALIGNED) \
> +               && defined(__ARMEL__))
>         #error FOO
>         #endif
>      }]
>

Ok by me. But please wait for a testsuite maintainer to approve.

regards
Ramana