On Fri, 14 Jun 2024, Andrew Pinski wrote:
> 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).
points-to should have that computed (conservatively). I'll see to
split out a ref_can_have_store_data_races (..) function.
Richard.
> 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
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
