On Fri, Jun 14, 2024 at 5:54 AM Richard Biener <rguent...@suse.de> wrote:
>
> Automatic arrays that are not address-taken should not be subject to
> store data races.

That seems conservative enough. Though I would think if the array
never escaped the function would be still correct and allow for more
arrays (but maybe that is not tracked).

Thanks,
Andrew Pinski

> This applies to OMP SIMD in-branch lowered
> functions result array which for the testcase otherwise prevents
> vectorization with SSE and for AVX and AVX512 ends up with spurious
> .MASK_STORE to the stack surviving.
>
> This inefficiency was noted in PR111793.
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.  Is my
> idea of store data races correct?  At least phiopt uses the same
> check but for example LIM doesn't special-case locals.
>
>         PR tree-optimization/111793
>         * tree-if-conv.cc (ifcvt_memrefs_wont_trap): For stores
>         that do not trap only consider -fstore-data-races when
>         the underlying object is not automatic or has its address
>         taken.
>
>         * gcc.dg/vect/vect-simd-clone-21.c: New testcase.
> ---
>  gcc/testsuite/gcc.dg/vect/vect-simd-clone-21.c | 16 ++++++++++++++++
>  gcc/tree-if-conv.cc                            | 13 +++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/vect/vect-simd-clone-21.c
>
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-simd-clone-21.c 
> b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-21.c
> new file mode 100644
> index 00000000000..49c52fb59bd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-21.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target vect_simd_clones } */
> +/* { dg-additional-options "-fopenmp-simd" } */
> +
> +#pragma omp declare simd simdlen(4) inbranch
> +__attribute__((noinline)) int
> +foo (int a, int b)
> +{
> +  return a + b;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 4 "vect" { target 
> i?86-*-* x86_64-*-* } } } */
> +/* if-conversion shouldn't need to resort to masked stores for the result
> +   array created by OMP lowering since that's automatic and does not have
> +   its address taken.  */
> +/* { dg-final { scan-tree-dump-not "MASK_STORE" "vect" } } */
> diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
> index c4c3ed41a44..974c614edf3 100644
> --- a/gcc/tree-if-conv.cc
> +++ b/gcc/tree-if-conv.cc
> @@ -934,14 +934,23 @@ ifcvt_memrefs_wont_trap (gimple *stmt, 
> vec<data_reference_p> drs)
>        if (DR_IS_READ (a))
>         return true;
>
> +      bool ok = flag_store_data_races;
> +      base = get_base_address (base);
> +      if (DECL_P (base)
> +         && auto_var_in_fn_p (base, cfun->decl)
> +         && ! may_be_aliased (base))
> +       /* Automatic variables not aliased are not subject to
> +          data races.  */
> +       ok = true;
> +
>        /* an unconditionaly write won't trap if the base is written
>           to unconditionally.  */
>        if (base_master_dr
>           && DR_BASE_W_UNCONDITIONALLY (*base_master_dr))
> -       return flag_store_data_races;
> +       return ok;
>        /* or the base is known to be not readonly.  */
>        else if (base_object_writable (DR_REF (a)))
> -       return flag_store_data_races;
> +       return ok;
>      }
>
>    return false;
> --
> 2.35.3

Reply via email to