Patchwork Adjust diag-scans in vect-tests to fix fails on AVX/AVX2

login
register
mail settings
Submitter Uros Bizjak
Date Dec. 12, 2011, 8:32 a.m.
Message ID <CAFULd4Yb+BQXTfsaJtasd=QtTzYQff7M5MbuypxfrrY-ZhkP2g@mail.gmail.com>
Download mbox | patch
Permalink /patch/130671/
State New
Headers show

Comments

Uros Bizjak - Dec. 12, 2011, 8:32 a.m.
Hello!

> This patch fixes dg-final scans in tests from vect.exp suite, which
> currently fail when avx2 is used.

using peeling" 2 "vect" { target {! vect_multiple_sizes} } } } */
+/* { dg-final { scan-tree-dump-times "Alignment of access forced
using peeling" 2 "vect" { xfail  vect_multiple_sizes} } } */
 /* { dg-final { cleanup-tree-dump "vect" } } */

Please do not add xfails through the patch, xfail means that a problem
was identified and will someday be fixed. In the above case, just add
target condition, no need for xfailed scan. If I'm not missing
simething, you can probably remove all introduced xfails, just add new
target conditions.

 # Return 1 if avx instructions can be compiled.

+proc check_effective_explicit_target_avx { } {
+  return [check_no_messages_and_pattern e_avx
"!__builtin_ia32_vzeroall" assembly {
+    void _mm256_zeroall (void)
+    {
+      __builtin_ia32_vzeroall ();
+    }
+  } "-O2" ]
+}

Please use

# Return true if we are compiling for AVX target.

proc check_avx_available { } {
    return [check_no_compiler_messages avx_available assembly {
        #ifndef __AVX__
        #error unsupported
        #endif
    } ""]
}

Uros.
Michael Zolotukhin - Dec. 12, 2011, 11 a.m.
I changed xfails to target-checks - for now I use common
vect_multiple_sizes (though it'll fail when wider vectors emerge).
Also, I changed AVX-check to the version Uros suggested. Please check
updated patch (attached).

As for vect_multiple_sizes_32B_16B and similar - isn't it too
target-specific? I think if we want to keep everything as general as
possible, we should have something like vect_1_vector_size_available,
vect_2_vector_sizes_available, etc.


New changelog:
2011-12-12  Michael Zolotukhin  <michael.v.zolotukhin@intel.com>

       * gcc.dg/vect/no-section-anchors-vect-31.c: Adjust diagnostic test to
       fix fail on AVX.
       * gcc.dg/vect/no-section-anchors-vect-66.c: Ditto.
       * gcc.dg/vect/no-section-anchors-vect-68.c: Ditto.
       * gcc.dg/vect/no-section-anchors-vect-69.c: Ditto.
       * gcc.dg/vect/no-vfa-vect-dv-2.c: Ditto.
       * gcc.dg/vect/pr45752.c: Ditto.
       * gcc.dg/vect/slp-perm-4.c: Ditto.
       * gcc.dg/vect/slp-perm-9.c: Ditto.
       * gcc.dg/vect/vect-33.c: Ditto.
       * gcc.dg/vect/vect-35.c: Ditto.
       * gcc.dg/vect/vect-6-big-array.c: Ditto.
       * gcc.dg/vect/vect-6.c: Ditto.
       * gcc.dg/vect/vect-91.c: Ditto.
       * gcc.dg/vect/vect-all-big-array.c: Ditto.
       * gcc.dg/vect/vect-all.c: Ditto.
       * gcc.dg/vect/vect-multitypes-1.c: Ditto.
       * gcc.dg/vect/vect-outer-4c.c: Ditto.
       * gcc.dg/vect/vect-outer-5.c: Ditto.
       * gcc.dg/vect/vect-over-widen-1.c: Ditto.
       * gcc.dg/vect/vect-over-widen-3.c: Ditto.
       * gcc.dg/vect/vect-over-widen-4.c: Ditto.
       * gcc.dg/vect/vect-peel-1.c: Ditto.
       * gcc.dg/vect/vect-peel-2.c: Ditto.
       * gcc.dg/vect/vect-peel-3.c: Ditto.
       * gcc.dg/vect/vect-reduc-pattern-1b.c: Ditto.
       * gcc.dg/vect/vect-reduc-pattern-1c.c: Ditto.
       * gcc.dg/vect/vect-reduc-pattern-2b.c: Ditto.
       * gcc.dg/vect/wrapv-vect-reduc-pattern-2c.c: Ditto.
       * gcc.dg/vect/no-section-anchors-vect-36.c: Adjust array size to fix
       fail on AVX.
       * gcc.dg/vect/no-section-anchors-vect-64.c: Ditto.
       * lib/target-supports.exp
(check_effective_target_vect_any_perm): New function.
       (check_avx_available): Ditto.
       (check_effective_target_vect_aligned_arrays): Add handling of AVX.
       (check_effective_target_vect_multiple_sizes): Ditto.


On 12 December 2011 12:32, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
>> This patch fixes dg-final scans in tests from vect.exp suite, which
>> currently fail when avx2 is used.
>
> --- a/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-31.c
> +++ b/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-31.c
> @@ -88,5 +88,6 @@ int main (void)
>
>  /* { dg-final { scan-tree-dump-times "vectorized 4 loops" 1 "vect" } } */
>  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned
> access" 0 "vect" } } */
> -/* { dg-final { scan-tree-dump-times "Alignment of access forced
> using peeling" 2 "vect" } } */
> +/* { dg-final { scan-tree-dump-times "Alignment of access forced
> using peeling" 2 "vect" { target {! vect_multiple_sizes} } } } */
> +/* { dg-final { scan-tree-dump-times "Alignment of access forced
> using peeling" 2 "vect" { xfail  vect_multiple_sizes} } } */
>  /* { dg-final { cleanup-tree-dump "vect" } } */
>
> Please do not add xfails through the patch, xfail means that a problem
> was identified and will someday be fixed. In the above case, just add
> target condition, no need for xfailed scan. If I'm not missing
> simething, you can probably remove all introduced xfails, just add new
> target conditions.
>
>  # Return 1 if avx instructions can be compiled.
>
> +proc check_effective_explicit_target_avx { } {
> +  return [check_no_messages_and_pattern e_avx
> "!__builtin_ia32_vzeroall" assembly {
> +    void _mm256_zeroall (void)
> +    {
> +      __builtin_ia32_vzeroall ();
> +    }
> +  } "-O2" ]
> +}
>
> Please use
>
> # Return true if we are compiling for AVX target.
>
> proc check_avx_available { } {
>    return [check_no_compiler_messages avx_available assembly {
>        #ifndef __AVX__
>        #error unsupported
>        #endif
>    } ""]
> }
>
> Uros.
Jakub Jelinek - Dec. 12, 2011, 11:10 a.m.
On Mon, Dec 12, 2011 at 03:00:52PM +0400, Michael Zolotukhin wrote:
> I changed xfails to target-checks - for now I use common
> vect_multiple_sizes (though it'll fail when wider vectors emerge).
> Also, I changed AVX-check to the version Uros suggested. Please check
> updated patch (attached).
> 
> As for vect_multiple_sizes_32B_16B and similar - isn't it too
> target-specific? I think if we want to keep everything as general as
> possible, we should have something like vect_1_vector_size_available,
> vect_2_vector_sizes_available, etc.

Depends on the test.  For some tests you don't need to distinguish in
between vect_multiple_sizes vs. !vect_multiple_sizes at all, the first
size will work out.  For other tests e.g. none could work out and thus
you'd see for each attempted vector sizes similar messages.
Then you can e.g. have tests with very small number of iterations that
will e.g. work out only for 16-byte vectors and not for 32-byte vectors.
In that case it depends on both vect_multiple_sizes vs.
!vect_multiple_sizes, the number of sizes attempted, but also which was
the first one, with -mprefer-avx128 you get different number of messages
from -mavx -mno-prefer-avx128, because in the former case it will not retry
with 32-byte vectors, while in the latter case it will start with 32-byte
vectors and retry with 16-byte vectors.

	Jakub
Ira Rosen - Dec. 12, 2011, 11:36 a.m.
gcc-patches-owner@gcc.gnu.org wrote on 12/12/2011 01:00:52 PM:

> I changed xfails to target-checks - for now I use common
> vect_multiple_sizes (though it'll fail when wider vectors emerge).
> Also, I changed AVX-check to the version Uros suggested. Please check
> updated patch (attached).
>
> As for vect_multiple_sizes_32B_16B and similar - isn't it too
> target-specific? I think if we want to keep everything as general as
> possible, we should have something like vect_1_vector_size_available,
> vect_2_vector_sizes_available, etc.

I think there is a difference between different vector sizes, and calling
it vect_X_vector_size_available is not sufficient. Your patch will cause
failures on ARM. It has two vector sizes, 16 and 8 bytes. E.g., vect-33.c
gets vectorized with the default vector size, and the alignment message
should be printed only once, and not twice as with your patch. So, it looks
like you need several vect_multiple_sizes_X.

Ira
Michael Zolotukhin - Dec. 12, 2011, 11:57 a.m.
> I think there is a difference between different vector sizes, and calling
> it vect_X_vector_size_available is not sufficient. Your patch will cause
> failures on ARM. It has two vector sizes, 16 and 8 bytes. E.g., vect-33.c
> gets vectorized with the default vector size, and the alignment message
> should be printed only once, and not twice as with your patch. So, it looks
> like you need several vect_multiple_sizes_X.

Probably we really need to have something like
vect_multiple_sizes_32B_16B - it'll help in this test. In this case we
could check if specific size is available, and it's crucial when array
sizes are so small, that they define the vector width to be used,
vect_multiple_sizes isn't sufficient for this, because the specific
size matters. I'll prepare a patch with such changes.

By the way, how could we check if '-mprefer-avx128' was specified from
target-supports.exp? Is there any global-variable for command line
options or something similar?

On 12 December 2011 15:36, Ira Rosen <IRAR@il.ibm.com> wrote:
>
>
> gcc-patches-owner@gcc.gnu.org wrote on 12/12/2011 01:00:52 PM:
>
>> I changed xfails to target-checks - for now I use common
>> vect_multiple_sizes (though it'll fail when wider vectors emerge).
>> Also, I changed AVX-check to the version Uros suggested. Please check
>> updated patch (attached).
>>
>> As for vect_multiple_sizes_32B_16B and similar - isn't it too
>> target-specific? I think if we want to keep everything as general as
>> possible, we should have something like vect_1_vector_size_available,
>> vect_2_vector_sizes_available, etc.
>
> I think there is a difference between different vector sizes, and calling
> it vect_X_vector_size_available is not sufficient. Your patch will cause
> failures on ARM. It has two vector sizes, 16 and 8 bytes. E.g., vect-33.c
> gets vectorized with the default vector size, and the alignment message
> should be printed only once, and not twice as with your patch. So, it looks
> like you need several vect_multiple_sizes_X.
>
> Ira
>
>
>
Jakub Jelinek - Dec. 12, 2011, 12:03 p.m.
On Mon, Dec 12, 2011 at 03:57:09PM +0400, Michael Zolotukhin wrote:
> By the way, how could we check if '-mprefer-avx128' was specified from
> target-supports.exp? Is there any global-variable for command line
> options or something similar?

I'd say try some very simple vectorized loop and check how it has
been vectorized.  Whether using %xmm or %ymm vectors.
Say
int a[1024], b[1024], c[1024];
void foo (void) { int i; for (i = 0; i < 1024; i++) a[i] = b[i] + c[i]; }
or so.

	Jakub
Ira Rosen - Dec. 12, 2011, 12:16 p.m.
Michael Zolotukhin <michael.v.zolotukhin@gmail.com> wrote on 12/12/2011
01:57:09 PM:

>
> By the way, how could we check if '-mprefer-avx128' was specified from
> target-supports.exp?
>

If I understand your question correctly, you can use check-flags (see
check_effective_target_arm_fp16_ok_nocache for example).

> Is there any global-variable for command line
> options or something similar?

flags

Ira
Jakub Jelinek - Dec. 12, 2011, 12:23 p.m.
On Mon, Dec 12, 2011 at 02:16:04PM +0200, Ira Rosen wrote:
> Michael Zolotukhin <michael.v.zolotukhin@gmail.com> wrote on 12/12/2011
> 01:57:09 PM:
> 
> >
> > By the way, how could we check if '-mprefer-avx128' was specified from
> > target-supports.exp?
> >
> 
> If I understand your question correctly, you can use check-flags (see
> check_effective_target_arm_fp16_ok_nocache for example).

The problem is that -mprefer-avx128/-mno-prefer-avx128 has a default
determined by -march/-mtune, some CPU tuning turn on -mprefer-avx128 by
default.

	Jakub

Patch

--- a/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-31.c
+++ b/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-31.c
@@ -88,5 +88,6 @@  int main (void)

 /* { dg-final { scan-tree-dump-times "vectorized 4 loops" 1 "vect" } } */
 /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned
access" 0 "vect" } } */
-/* { dg-final { scan-tree-dump-times "Alignment of access forced
using peeling" 2 "vect" } } */
+/* { dg-final { scan-tree-dump-times "Alignment of access forced